On Fri, Aug 11, 2017 at 7:05 AM, Matteo Merli <matteo.me...@gmail.com> wrote:
> I am ok in having rebased pull-requests and to allow the "Rebase & Merge" > option in github. > That can be helpful in cases in which there are multiple logical (though > related) commits for > which you want to keep separation. > For most cases (eg: PRs comments and incremental fixes on the PR) I think > the squash option > is still better, because the history of the PR is anyway available, with > all the comments and commits. > I agree. On Traffic Server project, we only use "Rebase & Merge", and reviewers are expected to check if the commits are reasonably separated / squashed. Actually most PRs contains only one commit, but sometimes the case happens, as you know: https://github.com/apache/incubator-pulsar/pull/524 And it's not just for commit history. In case of revert / cherry-pick / bisect, small commits are easy to deal with. > > My only concern is that once you chose that option and "rebase & merge" one > PR , it becomes your default, until you change it back. > So one has to really pay attention to not forget to go back to the squash > option. > Hmm, I didn't know that. That could be a problem. > > One other option would be to have a customized script to merge PRs. This > can help in enforce a uniform style in the > commit logs (as well as adding the name of the reviewers). For example, > this is what we use in BookKeeper: > https://github.com/apache/bookkeeper/blob/master/dev/bk-merge-pr.py > > It's a bit more involved than just hitting "Squash & merge" on github, but > nothing really major. > I guess this is the reason all PRs on BookKeeper project are marked as closed (not merged) and the commits page on GitHub doesn't have a link for the PRs. I'd rather use features on GitHub. > What do other people think about this? > Anyone? > > > Matteo > > On Tue, Aug 8, 2017 at 4:50 AM Masakazu Kitajo <mas...@apache.org> wrote: > > > Hi, > > > > I'd like to propose enabling "Rebase and merge"[1] as an option to merge > > pull requests. > > > > Currently, we have only one option, "Squash and merge". However, I'm not > a > > big fun of this option because it makes commit history too clean. Even if > > you made several meaningful commits on your branch, they would be > squashed > > when we merge the pull-request. > > > > What if we want to revert a part of the changes? > > What if we want to cherry-pick a part of changes? > > > > These sometimes happen, and in general, it's a good practice to keep > > commits small. > > > > As long as commits in a pull request seem meaningful, I think we should > > keep the commits separated. > > > > What do you think? > > > > [1] https://github.com/blog/2243-rebase-and-merge-pull-requests > > > > Masakazu > > > -- > Matteo Merli > <mme...@apache.org> > Masakazu
