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

Add --msg-filename option #39

Merged
merged 1 commit into from
Mar 29, 2018
Merged

Add --msg-filename option #39

merged 1 commit into from
Mar 29, 2018

Conversation

asottile
Copy link
Contributor

@asottile asottile commented Sep 20, 2017

The --msg-filename option enables the following:

  • Easier checking in a commit-msg hook without requiring a cat process.
  • Easy integration with http://pre-commit.com

This is related to #38

@jorisroovers jorisroovers force-pushed the master branch 2 times, most recently from 9430932 to 033978d Compare December 3, 2017 16:53
The --msg-filename option enables the following:
- Easier checking in a commit-msg hook without requiring a `cat` process.
- Easy ntegration with http://pre-commit.com
@asottile
Copy link
Contributor Author

asottile commented Dec 8, 2017

@jorisroovers I've rebased this branch (just noticed the conflicts -- sorry!)

@asottile
Copy link
Contributor Author

asottile commented Feb 6, 2018

@jorisroovers any feedback on this patch?

@pbregener
Copy link
Contributor

Can't you already do gitlint < commit_message.txt instead?

@asottile
Copy link
Contributor Author

asottile commented Feb 7, 2018

@pbregener that would require a shell

@jorisroovers
Copy link
Owner

Thanks for your interest in gitlint. I apologize it took me this look to get back to you - life and other projects get in the way sometimes :)

Definitely like the idea.
I think we could actually do something more generic here by calling the parameter --commit-message and supporting the @ syntax like many other tools do. Like so:

gitlint --commit-message "This is my commit message"
# Handy if you want to read in an env variable
gitlint --commit-message "$MESSAGE"

gitlint --commit-message @/path/to/my/commit/messagefile.txt

Thoughts?

Let me know if you still have time to work on this, if not, I'll see if I can squeeze it in.

@asottile
Copy link
Contributor Author

I've literally never seen a tool accept an argument like that. It should really be two separate options (either --commit-msg ... or --msg-filename ...)

The wacky @ syntax isn't compatible with #38 either :(

@jorisroovers
Copy link
Owner

Here's one example: https://github.com/jakubroztocil/httpie#request-items

Admittedly, the syntax not widely popular - but I've seen it used elsewhere.

Anyways, I'm good with this then. I'll merge it in - I'll update the docs and integration tests later (appreciate any PRs related to that).

@jorisroovers jorisroovers merged commit 941b76a into jorisroovers:master Mar 29, 2018
@asottile asottile deleted the msg_filename branch March 29, 2018 16:50
@asottile
Copy link
Contributor Author

I think I already added some tests for ya :)

@jorisroovers
Copy link
Owner

integration tests in the https://github.com/jorisroovers/gitlint/tree/master/qa directory, but all good - I can do that later.

@asottile
Copy link
Contributor Author

oh, I see -- sorry missed that :)

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.

3 participants