Settings

Theme

Show HN: GitCop – Automated Commit Message Validation for GitHub Pull Requests

gitcop.com

55 points by Gazler 11 years ago · 28 comments

Reader

akoeplinger 11 years ago

Can the bot comment include an example of a good commit message?

Telling users that "Commits must be in the following format: %{type}(%{scope}): %{description}" requires them to mentally parse and figure out what this means. Giving an example along the lines of "perf(backend): optimized db access" would make this easier.

  • GazlerOP 11 years ago

    This is a great idea, definitely something worth implementing. Thanks for the feedback.

weavejester 11 years ago

It's a very nice idea, but the validation rules are extremely limited.

Perhaps I'm wrong, but all of the rule options on the site look like they could have been implemented via regular expressions. Given the technical expertise of your intended customers, why not allow them to set a list of rules in the format: [regex, error-message], e.g.

    "^.{0,50}\n"         "The subject should be 50 characters or under"
    "^.+\n(.{0,72}\n)*$" "The body should have lines of 72 characters or under"
    "^\p{Upper}"         "The subject should be capitalized"
  • GazlerOP 11 years ago

    I think doing it like that is a neat idea. Perhaps there could be an "advanced" page which is as you describe and the current method could be the "simple" version?

eLobato 11 years ago

This is quite cool, and actually something I'll propose to use at some http://github.com/theforeman repos.

I didn't really understood the format rules, so if the author posted this, a help section would help. I try to enforce

    '%(type) #xxxx(: | -) %{description}'
How would you do that? #xxxx is just an issue number, so it can be #23891, and after that I want a separator from the description, semi colon or hyphen are fine. Probably %{scope} can be used but I didn't really understand it from the inline help, sorry!
  • GazlerOP 11 years ago

    Hi, thanks for the feedback, it would be great to be used by a project like Foreman!

    Currently the string for the format only allows the 3 defined variables, type, scope and description. I have been using this style at https://github.com/Gazler/changex/commits/master to give you an idea of how it works.

    I was concerned that the format string might be a little weird to get from the help. I will try and clear this up a bit.

    I have attempted to build the string that you requested, currently an or separator is not supported, but I can take a look at adding it in.

    Here are the format strings that I think match your use case. Using a colon as a separator:

        test string: "Fixes #1234: foo"
        format:      "%{type} #%{scope}: %{description}"
        [type: "Fixes", scope: "1234", description: "foo"]
    
    Using a hyphen as a separator:

        test string: "Fixes #1234 - foo"
        format:      "%{type} #%{scope} - %{description}"
        [type: "Fixes", scope: "1234", description: "foo"]
    
    I hope this makes sense, let me know if you get it working!
    • eLobato 11 years ago

      Interesting, that makes sense. I'll check if the rest of the team would be ok with forcing commits to use colon as a separator until that feature is added. :)

      Still, what does %{scope} actually catch? Anything that's not %{type}, %{description} or an explicit character?

      • GazlerOP 11 years ago

        Glad it made sense!

        So the match is exact between the non %{..} characters. The idea for scope is the context of the commit, in my libraries I use the name of the class as the scope.

        For example with the default format string:

            test string: "fix(User): ensure email address is required"
            format:      "%{type}(%{scope}): %{description}"
            [type: "fix", scope: "User", description: "ensure email address is required"]
        
        So any literal characters are matched exactly and anything inside the capturing %{...} is assigned to that variable.

        This does fall over in the following case:

            "fix():(User): ensure email address is required"
            "%{type}(%{scope}): %{description}"
            [type: "fix", scope: "):(User", description: "ensure email address is required"]
        
        It is certainly a candidate for improvement in the future.
Myrmornis 11 years ago

After the PR is opened is too late to warn about commit messages. They can't be changed without a destructive push to the remote, which is generally a no-no for a public branch, and when using github, closes the PR automatically. The correct place to do this is, I believe, in the `commit-msg` hook: http://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks

iandanforth 11 years ago

Slightly off topic - are there any good plugins/tools that remind you to commit frequently? I'd love something like this for sublime.

akoeplinger 11 years ago

Have you thought about opening this up for more than just commit messages?

Many projects need to check a number of things on PRs, e.g. enforce coding style, bugfix has a corresponding test, user signed the CLA, owner of %{scope} is notified for review, etc. That would obviously need a way to tie into external/custom tools as everybody validates these things differently.

  • GazlerOP 11 years ago

    Thanks for the ideas.

    I think there are several avenues that could be explored with this. I wanted to stay away from explicitly checking the code initially, as there are other tools aimed at that specific task, however there is no reason something like this couldn't be added into GitCop at a later date.

    I had the idea of checking that the user exists in a list of users by specifying a JSON list of users. This means that it could be used to check if a CLA has been signed, but there are other applications too. Does this seem like a reasonable way to do it?

    I like the idea of ensuring a test has been added, not sure how to tackle it, but it is certainly worth keeping in mind.

    • akoeplinger 11 years ago

      I think what I was aiming at is integration with other tools, rather than adding those checks directly into GitCop (as you said, there are already services that check code, validate CLA etc).

      Or maybe provide me with a Travis-like sandbox where I can run my checks with whatever linter/validator/custom code I want and report the results back?

      Just fleshing out some ideas here :)

tokenrove 11 years ago

I like the idea, but for projects that aren't open source, why not just mandate the use of a git commit-msg hook instead?

  • GazlerOP 11 years ago

    A git hook would certainly work for this as you described and I would certainly encourage people to investigate that route if it is appropriate for them. http://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks

    The goal for GitCop is to be fast and easy to set up in a way that integrates well into a GitHub pull request based flow.

    • rurounijones 11 years ago

      By that point isn't it too late? Or do you expect people to commit --amend and force push?

      • GazlerOP 11 years ago

        It is common in many flows to expect people to do a force push if something is wrong with their pull request, e.g. having 15 commits that look like:

            Try stuff
            Another prototype
            Fix something
            ...
            Revert prototype
        
        It is fairly common (in my experience) for the contributor to be asked to squash those commits.

        In my opinion, the more guards in place the better.

      • dmethvin 11 years ago

        I think that's a key point, the earlier you can catch the problem the less frustrating for both sides. There is https://www.npmjs.org/package/commitplease for commit hooks but it's still handy to have something audit pull requests.

tjbiddle 11 years ago

Looks clean, has plenty of features, and I love the pricing schema (Unlimited for open source projects - Always been a fan of this trend).

Great work!

thestonefox 11 years ago

What an awesome service, I'm signing up my dev team for this right now! Worth the money!

  • GazlerOP 11 years ago

    Thanks for the feedback, I hope your dev team find it as valuable as you do.

dmooney1 11 years ago

I'd love it if you could make this work with Atlassian Stash behind a firewall.

  • mindsocket 11 years ago

    Stash has a number of integration points where similar checks could be applied, the most similar to GitCop being a merge check. That said, a pre-receive hook would make a lot more sense.

    Disclosure: I work for Atlassian

  • GazlerOP 11 years ago

    I'm not too familiar with Atlassian Stash, I will have a search around and see how plausible it would be to integrate.

jimmyboyb 11 years ago

I have been looking for something like this

warrenmcwin 11 years ago

looks like Robocop has finally joined the Pull Police. nice job!

Keyboard Shortcuts

j
Next item
k
Previous item
o / Enter
Open selected item
?
Show this help
Esc
Close modal / clear selection