In general I'm for Squashing +1. In some exceptional cases I would have a small number of commits to facilitate easier reviewing (like in AVRO-2117).
Niels. On Thu, Oct 25, 2018 at 7:31 PM Doug Cutting <[email protected]> wrote: > > I'm +1 for squashing. > > Doug > > On Tue, Oct 23, 2018 at 2:35 PM Michael A. Smith <[email protected]> > wrote: > > > I’m generally in favor of linear commits iff the committers can be trusted > > to follow good git hygiene in general, like with small, proprietary > > projects with strong leadership. > > > > With Avro, with its variegated multilang repo and commits coming from the > > community at large, I’m not sure if linear-rebase or squash is a great > > idea. > > > > If you think we can put an automated check in place to make sure commits in > > PRs follow good hygiene by always having ticket tags, signed commits, > > “real” authors, or whatever criteria makes sense, then I’m all for it. > > > > Cheers, > > Michael Smith/kojiromike > > > > On Tue, Oct 23, 2018 at 17:19 Driesprong, Fokko <[email protected]> > > wrote: > > > > > Hi all, > > > > > > Allow me to start the great debate. > > > > > > [image: image.png] > > > > > > I would like to get the Avro's dev-community's opinion on the merge > > > strategy of pull requests. By default Github supports three options: > > > - Create a merge commit > > > - Squash and merge > > > - Rebase and merge > > > https://help.github.com/articles/about-merge-methods-on-github/ > > > > > > Currently I see the PR's being merged into master using the merge commit > > > (with some exceptions). From my perspective this has two major > > > disadvantages: > > > - It will add ugly merge commits on top of master, which don't add any > > > value. > > > - The git commit tree history isn't lineair. For example, a PR that has > > > been merged recently <https://github.com/apache/avro/pull/270>, added a > > > commit back in 19 dec 2017. > > > - It makes it tricky to revert commits, since a PR merged using a merge > > > commit might add one or more commits over the history of master. > > > > > > Therefore I would only allow two merge strategies: > > > - Squash and merge: This will squash all the commits of the PR into one > > > single commit, and push it on top of master. This should be the default > > > strategy. > > > - Rebase and merge: This can be used for very big PR's, in which you > > don't > > > want to squash the original. > > > > > > Github allows us to disable merge-options. My suggestion would be to > > > disable the merge commit option, but I'd like to get the community's > > > opinion on it. > > > > > > Cheers, Fokko > > > > > > > > > > > -- Best regards / Met vriendelijke groeten, Niels Basjes
