Re: Merge strategy
I’m ok with squashing, but if the PR only has one commit in it, then a rebase and merge seems appropriate. Dan > On Oct 23, 2018, at 5:19 PM, Driesprong, Fokko wrote: > > Hi all, > > Allow me to start the great debate. > > > > 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/ > <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 > > -- Daniel Kulp dk...@apache.org <mailto:dk...@apache.org> - http://dankulp.com/blog <http://dankulp.com/blog> Talend Community Coder - http://talend.com <http://coders.talend.com/>
Re: Merge strategy
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 wrote: > > I'm +1 for squashing. > > Doug > > On Tue, Oct 23, 2018 at 2:35 PM Michael A. Smith > 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 > > 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
Re: Merge strategy
I'm +1 for squashing. Doug On Tue, Oct 23, 2018 at 2:35 PM Michael A. Smith 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 > 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 > > > > > > >
Merge strategy
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
