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