https://github.com/marketplace/actions/gs-commit-message-checker
On Tue, Aug 4, 2020 at 10:25 AM Robert Bradshaw <[email protected]> wrote: > +1, thanks for the reminder. > > This should be really easy to automate, using > https://developer.github.com/webhooks/event-payloads/#pull_request to > give a warning when the change history is not sufficiently "clean." > I'm not sure where to host this though (or if it could be integrated > into jenkins--basically I'd just want to run a Python script with the > PR number (or better, just point to the local git repo and have the > master's commit handy) as another precommit). > > On Tue, Aug 4, 2020 at 10:10 AM Rui Wang <[email protected]> wrote: > > > > +1 thanks Alexey. > > > > My apologies that I merged such a case recently (but not intentionally). > I tried to use the "squash and merge" button with a consolidated commit > message. After clicking the button, github showed "failed to merge" and > gave a retry button, and after clicking that retry button, github magically > switched to "create merge commit" approach thus merged some fixup commits > to the main branch. > > > > This is a rare case (I only encountered once). But I will pay more > attention next time. I could ask PR authors to squash their commits before > merging when it is possible. > > > > > > -Rui > > > > On Tue, Aug 4, 2020 at 9:40 AM Alexey Romanenko < > [email protected]> wrote: > >> > >> 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]> 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 > >> > >> >
smime.p7s
Description: S/MIME Cryptographic Signature
