+1, thanks, Alexey. Also a reminder from the contributor guide: do not use the default GitHub commit message for merge commits, which looks like:
Merge pull request #1234 from some_user/transient_branch_name Instead, add the commit message into the subject line, for example: "Merge pull request #1234: [BEAM-7873] Fix the foo bizzle bazzle". On Tue, Aug 4, 2020 at 7:13 AM Alexey Romanenko <[email protected]> wrote: > Hi all, > > I’d like to attract your attention regarding our Git commit history and > related issue. A while ago I noticed that it started getting not very clear > and quite verbose comparing to how it was before. We have quite significant > amount of recent commits like “fix”, “address comments”, “typo”, > “spotless”, etc. Most of them also doesn’t contain Jira Tag as a prefix and > actually is just supplementary commits to “main” and initial commit of PR, > added after several PRs review rounds. > > AFAIR, we already had several discussion in the past about this topic and > we agreed that we should avoid such commits in a final merge and have only > one (in most cases) or several (if necessary) logical commits that should > be atomic and properly explain what they do. > > Why these “tiny" commits are bad practice? Just several main reasons: > - They pollute our git repository history and don’t give any additional > and useful further information; > - They are not atomic and we can’t easily revert (rollback) this > supplementary commit since the state of the build before was likely broken > or had incorrect behaviour. So, in this case, the whole set of PRs commits > should be reverted which is not convenient and error-prone. It’s also > expected that all checks were green before merging a PR (take a part flaky > tests). > - They are not informative in terms of commit message. So it makes more > hard to identify Git annotated code and how the lines of code are related > together. > > Following this, I just want to briefly remind our Committers rules > regarding PR merging [1]. > Every commit: > - should do one thing and reflect it in commit message; > - should contain Jira Tag; > - all “fixup” and “address comments” type of commits should be squashed by > author or committer before merging. > > Please, pay attention on what is finally committed and merged into our > repository and it should help to keep our commit history clear, which will > be transferred to saving a time of other developers in the end. > > [1] https://beam.apache.org/contribute/committer-guide/#finishing-touches > > Regards, > Alexey >
