Yes, good point, thanks Valentyn.
> On 4 Aug 2020, at 18:29, Valentyn Tymofieiev <[email protected]> wrote:
>
> +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]
> <mailto:[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
> <https://beam.apache.org/contribute/committer-guide/#finishing-touches>
>
> Regards,
> Alexey
