Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bin: allow file:/// urls to files containing commit messages #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Oct 27, 2016

add the usage model of core-validate-commit file:///somefile…

The file needs to approximate the output of git show --quiet --format=medium ${sha}

Semver: minor

@srl295
Copy link
Member Author

srl295 commented Oct 27, 2016

  • TODO lint

srl295 added a commit to srl295/core-validate-commit that referenced this pull request Oct 27, 2016
Add instructions and a hook to call core-validate-commit from
git commits.

Depends on nodejs#11

Known issues:
- leaves ${TMPF} around (temporary file)
- probably will reject `fixup:` type commit logs
- may be too strict if someone is just doing a simple
'work in progress' kind of local branch

Larger improvement:
- fold the entire hook script into a mode, and/or an alternate
script entrypoint into core-validate-commit. This would drop
requirements on sh, sed, grep…

Fixes: nodejs#12
srl295 added a commit to srl295/core-validate-commit that referenced this pull request Oct 27, 2016
Add instructions and a hook to call core-validate-commit from
git commits.

Depends on nodejs#11

Known issues:
- leaves ${TMPF} around (temporary file)
- probably will reject `fixup:` type commit logs
- may be too strict if someone is just doing a simple
'work in progress' kind of local branch

Larger improvement:
- fold the entire hook script into a mode, and/or an alternate
script entrypoint into core-validate-commit. This would drop
requirements on sh, sed, grep…

Fixes: nodejs#12
@not-an-aardvark
Copy link
Contributor

Would it be possible to just allow a filepath, rather than a file:/// URL? A filepath seems like it would be easier to use.

@srl295
Copy link
Member Author

srl295 commented Oct 27, 2016

@not-an-aardvark right now the implementation is that it checks whether it's a valid URL, otherwise assumes it's a sha.

I think that makes sense, however, I wasn't sure what the best parameter option would be. I think a related question here is about the format of the input. right now it needs to be equivalent to git show …, see what I did in #13 to make the input acceptable.

The current use cases (sha, url) are the higher runner cases for command line usage. Probably this should be handled by just two additional paramteres:

$ cat > /some/fileA <<EOF
commit 9236b187d1bc49aea9205a52b516f1c5
Author: My Self <me@example.com>
Date:    Jan 1 00:00:00 1970 -0000

    subsys: something

    * stuff

    metadata…
EOF

$ core-validate-commit --commit-info /some/fileA

$ cat > /some/fileB <<EOF
subsys: something

* stuff

metadata…
EOF

$ core-validate-commit --commit-msg /some/fileB

expecting fileA to have the full git show output (indented), but fileB to just have the commit message.

( --commit-info would correspond to the behavior in this PR, the other isn't implemented. )

@evanlucas
Copy link
Contributor

It may make sense to just make the three different types of inputs (sha, url, file) require a different flag for each one. It would definitely make it less ambiguous.

Adsada2205

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants