I agree with Ash that it's a committer's task to check the commit name. But I personally was deceived by a discrepancy between PR name and commit title and merged the PR with "wrong" commit message.
That's where I agree with Jarek: we should help ourselves. If "red check" will make people correct the commit / PR title then there will be less work for us and fewer mistakes made by us. Also, semantic commits have a nice side effect: they teach about commit messages. I think having a tool to check and teach is better having a "you should follow this" link, which in most cases is ticked without clicking the link (btw. should we measure it?). One of my reason to suggest it was a conventional changelog that could be auto-generated. As Jarek mentioned currently it's mostly done by @Kaxil and it would be interesting to hear what he thinks about it. T. On Sun, Apr 26, 2020 at 2:44 PM Jarek Potiuk <jarek.pot...@polidea.com> wrote: > I think you pointed out the exact things I thought are important and could > be automated. I think those are the very things committing checks for. > > I think we could benefit from 1, 2, 3, 4, 6 (6. with exceptions indeed but > a warning or a way to mark an exception would be nice - similarly as we do > with pylint). > I certainly did not want to improve automatically on the 5 (yet) and 7 > (here it's much more of a convention we agree between the committers - > whether the body should be optional - I think it should and whether it > should be opt-out rather than opt-in - I thin it should be opt-out). > > There are quite a few commits currently with ... when you look at > the commit log in Github for one (because they do not obey the subject > length) - I picked the ones without JIRAs - still even without JIRAs > sometimes the subject is too long: > > - > > https://github.com/apache/airflow/commit/d883ff49ca2841f91ab7e0ab98204d5ad271473b > - > > https://github.com/apache/airflow/commit/bc230a9711fec2004e20f46aee22fb44c7461b6c > - > > https://github.com/apache/airflow/commit/fa262c12f87102a7ae1abb11ea7f0d5e8be0de47 > > However - this is secondary. It was merely a comment on the possible > completion of the "semantic convention" approach. This is the main subject. > > I think the main idea behind the semantic commit/PR is the prefix is that > it allows for much easier and consistent ChangeLog generation. For example > in Angular you have > https://github.com/angular/angular/blob/master/CHANGELOG.md which is > generated automatically including breaking changes etc. I think it's > mainly Kaxil's work now to prepare the changelog and group the changes into > separate buckets, so Kaxil - maybe your opinion is important here. If there > is a way everyone as committers and contributors we can do to make release > manager's job easier - I think we should do it. > > BTW. The convention is easy to follow without any tools. However commitizen > has the nice feature of also guiding new users - it provides a nice > explanation of the types you have defined in the project and guide the new > users how to write a good commit. I think it might be really nice touch for > our "welcoming community" approach. See the 5 minutes video about it: > > https://egghead.io/lessons/javascript-writing-conventional-commits-with-commitizen > > J. > > J. > > > > On Sun, Apr 26, 2020 at 2:13 PM Ash Berlin-Taylor <a...@apache.org> wrote: > > > My main objection is this is trying to apply a technical solution to a > > people+English problem. This feels like just one extra step to have > > commiters to do, when we as committers can very easily correct this in > > Github whilst reviewing/before merging. > > > > That said, can you point at any examples of recent commits that you > > think would have been clearer as a result of using? > > > > (Also a significant proportion of commits from form a git gui or an ide, > > so cz-cli won't help those users.) > > > > The "good commit messages" we already link to > > https://chris.beams.io/posts/git-commit/ has these points > > > > 1. Separate subject from body with a blank line > > 2. Limit the subject line to 50 characters > > 3. Capitalize the subject line > > 4. Do not end the subject line with a period > > 5. Use the imperative mood in the subject line > > 6. Wrap the body at 72 characters > > 7. Use the body to explain what and why vs. how > > > > 2 we _could_ enforce, but it is not a hard-and-fast rule. 5 and 7 is > > almost impossible for a computer to enforce. 6 always has exceptions. > > The most important ones is 7, and that is the hardest to programitcally > > enforce. > > > > -a > > > > On Apr 26 2020, at 11:30 am, Jarek Potiuk <jarek.pot...@polidea.com> > > wrote: > > > > > I think it's a very good idea to use it. We already discussed that we > > > should have some improvements in the way we write commits - and why to > > > come up with our own conventions if we can adopt one that already > > > exists and has set of nice tools available. > > > > > > As usual, I think automation is a key - as it might make lives of > > > committers a bit easier. There are already a number of tools that we > > > could use together with such convention, as both pre-commits and a bot > > > in Github. > > > There are quite a few tools that embraced the concept of semantic > > > pr/semantic commits and I have heard good words about them from other > > > open-source projects. I've heard especially good words about > > > commitizen CLI, that could work hand-in-hand with semantic > > > commits/PRs: > > > > > > https://github.com/commitizen/cz-cli > > > > > > One of the things it has it also integrates with commit lint where we > > > could write our own rules and make them more meaningful > > > https://commitlint.js.org/#/ > > > > > > Also, there are ready-to-use changelog generators that we can use (for > > > example https://github.com/commitizen/cz-conventional-changelog ) > > > > > > Those are tools coming from the nodejs world, but I do not see a big > > > problem with using them (of course trying them out first) - since we > > > can now connect it via pre-commit, it should be easy to add all that > > > to our toolbox. > > > > > > J. > > > > > > > > > On Sun, Apr 26, 2020 at 10:44 AM Ash Berlin-Taylor <a...@apache.org> > > wrote: > > >> > > >> I agree that many commit messages are often lacking but I'm not a fan > > >> of that the prefix style that app requires, - plus I think it would > > >> still be possible to have unhelpful PR titles just with 'fix:' > prefixed. > > >> > > >> Is rather we as commiters updated the pr subjects when reviewing. The > > >> rule I try to follow is to (mentally) prefix the message with "When > > >> this commit is applied it will ..." > > >> > > >> -a > > >> > > >> On 26 April 2020 09:34:56 BST, Tomasz Urbaszek <turbas...@apache.org> > > wrote: > > >> >Hi all! > > >> > > > >> >Sometimes it happens that pull requests or commits have not so > > >> >meaningful messages and it's hard to say what's exactly going on. > > >> >So I am wondering if we would like to consider using semantic pull > > >> >request: https://github.com/zeke/semantic-pull-requests > > >> > > > >> >Since we are using Github it should be pretty easy to add: > > >> >https://github.com/apps/semantic-pull-requests > > >> > > > >> >Of course, it does not solve the problem of "pr message" but > > >> >definitely it raises attention about it. On the other hand, it should > > >> >also help with publishing changelogs. Personally I like this approach > > >> >and I used to use it before joining Airflow. > > >> > > > >> >Happy to see what you think about it. And sorry if it was decided > some > > >> >ago that Airflow won't follow it. > > >> > > > >> >Cheers, > > >> >Tomek > > > > > > > > > > > > -- > > > > > > Jarek Potiuk > > > Polidea | Principal Software Engineer > > > > > > M: +48 660 796 129 > > > > > > > > -- > > Jarek Potiuk > Polidea <https://www.polidea.com/> | Principal Software Engineer > > M: +48 660 796 129 <+48660796129> > [image: Polidea] <https://www.polidea.com/> >
