You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Sean Busbey <bu...@apache.org> on 2019/04/23 14:33:37 UTC
[DISCUSS] Reminder: please use 'rebase' or 'squash' when accepting
PRs via github
Folks,
Looking at history for the current master branch, we had several PRs
accepted over the last week where the committer used the "merge
commit" option. As a reminder, previous consensus was that we would
avoid this since it makes the history harder to follow.
Please ensure you are selecting either "rebase commits" or "squash
commits" when accepting a PR.
I have filed INFRA-18264 to disable the merge commit option.
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Peter Somogyi <ps...@apache.org>.
> How to we weigh these competing concerns?
When a contributor adds multiple commits to a pull request we can use
"Squash and merge". Therefore the pull request will land on the target
branch as a single commit which can be cherry-picked to other branches as
we did previously.
On Tue, Apr 23, 2019 at 7:51 PM Nick Dimiduk <nd...@gmail.com> wrote:
> > I prefer multiple commits in a single PR compared to force pushing to
> feature branch because it makes incremental reviewing simpler.
>
> I agree incremental commits make reviewing simpler. However, we have a long
> tradition of 1 JIRA, 1 commit. This makes it easier for release managers to
> conduct what still falls to manual business of (1) manually verifying the
> branch history and JIRA fixVersions and the CHANGES.txt all align and (2)
> selectively reverting changes identifies as having caused issue with a
> release candidate.
>
> How to we weigh these competing concerns?
>
> On Tue, Apr 23, 2019 at 6:21 PM Nick Dimiduk <nd...@gmail.com> wrote:
> >
> > > I am +1 for linear commit history.
> > >
> > > Does the “squash” option give the committer enough control over the
> > commit
> > > message format and structure? I personally prefer to perform all of my
> > > commit rewriting locally, verify it locally, and then force-push the
> > > desired history to the feature branch before using the “rebase” variant
> > of
> > > the button. Maybe that process is not necessary with the “squash”
> option?
> > >
> > > Thanks,
> > > Nick
> > >
> > > On Tue, Apr 23, 2019 at 7:33 AM Sean Busbey <bu...@apache.org> wrote:
> > >
> > > > Folks,
> > > >
> > > > Looking at history for the current master branch, we had several PRs
> > > > accepted over the last week where the committer used the "merge
> > > > commit" option. As a reminder, previous consensus was that we would
> > > > avoid this since it makes the history harder to follow.
> > > >
> > > > Please ensure you are selecting either "rebase commits" or "squash
> > > > commits" when accepting a PR.
> > > >
> > > > I have filed INFRA-18264 to disable the merge commit option.
> > > >
> > >
> >
>
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Nick Dimiduk <nd...@gmail.com>.
> I prefer multiple commits in a single PR compared to force pushing to
feature branch because it makes incremental reviewing simpler.
I agree incremental commits make reviewing simpler. However, we have a long
tradition of 1 JIRA, 1 commit. This makes it easier for release managers to
conduct what still falls to manual business of (1) manually verifying the
branch history and JIRA fixVersions and the CHANGES.txt all align and (2)
selectively reverting changes identifies as having caused issue with a
release candidate.
How to we weigh these competing concerns?
On Tue, Apr 23, 2019 at 6:21 PM Nick Dimiduk <nd...@gmail.com> wrote:
>
> > I am +1 for linear commit history.
> >
> > Does the “squash” option give the committer enough control over the
> commit
> > message format and structure? I personally prefer to perform all of my
> > commit rewriting locally, verify it locally, and then force-push the
> > desired history to the feature branch before using the “rebase” variant
> of
> > the button. Maybe that process is not necessary with the “squash” option?
> >
> > Thanks,
> > Nick
> >
> > On Tue, Apr 23, 2019 at 7:33 AM Sean Busbey <bu...@apache.org> wrote:
> >
> > > Folks,
> > >
> > > Looking at history for the current master branch, we had several PRs
> > > accepted over the last week where the committer used the "merge
> > > commit" option. As a reminder, previous consensus was that we would
> > > avoid this since it makes the history harder to follow.
> > >
> > > Please ensure you are selecting either "rebase commits" or "squash
> > > commits" when accepting a PR.
> > >
> > > I have filed INFRA-18264 to disable the merge commit option.
> > >
> >
>
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Peter Somogyi <ps...@apache.org>.
The committer has full control over commit message on squash commits. By
default, it will use PR title for first row and list of commit messages in
the commit message description.
I prefer multiple commits in a single PR compared to force pushing to
feature branch because it makes incremental reviewing simpler.
Peter
On Tue, Apr 23, 2019 at 6:21 PM Nick Dimiduk <nd...@gmail.com> wrote:
> I am +1 for linear commit history.
>
> Does the “squash” option give the committer enough control over the commit
> message format and structure? I personally prefer to perform all of my
> commit rewriting locally, verify it locally, and then force-push the
> desired history to the feature branch before using the “rebase” variant of
> the button. Maybe that process is not necessary with the “squash” option?
>
> Thanks,
> Nick
>
> On Tue, Apr 23, 2019 at 7:33 AM Sean Busbey <bu...@apache.org> wrote:
>
> > Folks,
> >
> > Looking at history for the current master branch, we had several PRs
> > accepted over the last week where the committer used the "merge
> > commit" option. As a reminder, previous consensus was that we would
> > avoid this since it makes the history harder to follow.
> >
> > Please ensure you are selecting either "rebase commits" or "squash
> > commits" when accepting a PR.
> >
> > I have filed INFRA-18264 to disable the merge commit option.
> >
>
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by William Shen <wi...@marinsoftware.com>.
Should we decide when to use squash and merge, and when to use rebase?
IMO, we should probably always use squash and merge, and guarantee a single
commit per PR landing in master.
Practically, there would be no difference between the two if the PR only
contains a single commit. However, I think we may start seeing multiple
commits in a single PR as we move code review to PR-based.
On Tue, Apr 23, 2019 at 9:43 AM Peter Somogyi <ps...@apache.org> wrote:
> > The committer then has control over the
> commit message, though it does break things into a "subject" box and a
> "body" box. the subject is placed first in the message followed by two
> newlines and then the body.
>
> I just double checked this on PR 181, it gives 2 boxes for commit message:
> title and description when doing squash merge.
>
> On Tue, Apr 23, 2019 at 6:39 PM Sean Busbey <bu...@apache.org> wrote:
>
> > IIRC, the squash option is only available when GitHub thinks the
> > rebase part will be clean. The committer then has control over the
> > commit message, though it does break things into a "subject" box and a
> > "body" box. the subject is placed first in the message followed by two
> > newlines and then the body.
> >
> > committers always have the option of handling the PR committing
> > locally. Just please remember to include a note in the message to
> > close the PR[1]. Or manually go close the PR afterwards with a note
> > about when it was merged.
> >
> > [1]:
> > http://hbase.apache.org/book.html#_close_related_github_prs
> >
> > On Tue, Apr 23, 2019 at 11:21 AM Nick Dimiduk <nd...@gmail.com>
> wrote:
> > >
> > > I am +1 for linear commit history.
> > >
> > > Does the “squash” option give the committer enough control over the
> > commit
> > > message format and structure? I personally prefer to perform all of my
> > > commit rewriting locally, verify it locally, and then force-push the
> > > desired history to the feature branch before using the “rebase” variant
> > of
> > > the button. Maybe that process is not necessary with the “squash”
> option?
> > >
> > > Thanks,
> > > Nick
> > >
> > > On Tue, Apr 23, 2019 at 7:33 AM Sean Busbey <bu...@apache.org> wrote:
> > >
> > > > Folks,
> > > >
> > > > Looking at history for the current master branch, we had several PRs
> > > > accepted over the last week where the committer used the "merge
> > > > commit" option. As a reminder, previous consensus was that we would
> > > > avoid this since it makes the history harder to follow.
> > > >
> > > > Please ensure you are selecting either "rebase commits" or "squash
> > > > commits" when accepting a PR.
> > > >
> > > > I have filed INFRA-18264 to disable the merge commit option.
> > > >
> >
>
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Peter Somogyi <ps...@apache.org>.
> The committer then has control over the
commit message, though it does break things into a "subject" box and a
"body" box. the subject is placed first in the message followed by two
newlines and then the body.
I just double checked this on PR 181, it gives 2 boxes for commit message:
title and description when doing squash merge.
On Tue, Apr 23, 2019 at 6:39 PM Sean Busbey <bu...@apache.org> wrote:
> IIRC, the squash option is only available when GitHub thinks the
> rebase part will be clean. The committer then has control over the
> commit message, though it does break things into a "subject" box and a
> "body" box. the subject is placed first in the message followed by two
> newlines and then the body.
>
> committers always have the option of handling the PR committing
> locally. Just please remember to include a note in the message to
> close the PR[1]. Or manually go close the PR afterwards with a note
> about when it was merged.
>
> [1]:
> http://hbase.apache.org/book.html#_close_related_github_prs
>
> On Tue, Apr 23, 2019 at 11:21 AM Nick Dimiduk <nd...@gmail.com> wrote:
> >
> > I am +1 for linear commit history.
> >
> > Does the “squash” option give the committer enough control over the
> commit
> > message format and structure? I personally prefer to perform all of my
> > commit rewriting locally, verify it locally, and then force-push the
> > desired history to the feature branch before using the “rebase” variant
> of
> > the button. Maybe that process is not necessary with the “squash” option?
> >
> > Thanks,
> > Nick
> >
> > On Tue, Apr 23, 2019 at 7:33 AM Sean Busbey <bu...@apache.org> wrote:
> >
> > > Folks,
> > >
> > > Looking at history for the current master branch, we had several PRs
> > > accepted over the last week where the committer used the "merge
> > > commit" option. As a reminder, previous consensus was that we would
> > > avoid this since it makes the history harder to follow.
> > >
> > > Please ensure you are selecting either "rebase commits" or "squash
> > > commits" when accepting a PR.
> > >
> > > I have filed INFRA-18264 to disable the merge commit option.
> > >
>
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Sean Busbey <bu...@apache.org>.
IIRC, the squash option is only available when GitHub thinks the
rebase part will be clean. The committer then has control over the
commit message, though it does break things into a "subject" box and a
"body" box. the subject is placed first in the message followed by two
newlines and then the body.
committers always have the option of handling the PR committing
locally. Just please remember to include a note in the message to
close the PR[1]. Or manually go close the PR afterwards with a note
about when it was merged.
[1]:
http://hbase.apache.org/book.html#_close_related_github_prs
On Tue, Apr 23, 2019 at 11:21 AM Nick Dimiduk <nd...@gmail.com> wrote:
>
> I am +1 for linear commit history.
>
> Does the “squash” option give the committer enough control over the commit
> message format and structure? I personally prefer to perform all of my
> commit rewriting locally, verify it locally, and then force-push the
> desired history to the feature branch before using the “rebase” variant of
> the button. Maybe that process is not necessary with the “squash” option?
>
> Thanks,
> Nick
>
> On Tue, Apr 23, 2019 at 7:33 AM Sean Busbey <bu...@apache.org> wrote:
>
> > Folks,
> >
> > Looking at history for the current master branch, we had several PRs
> > accepted over the last week where the committer used the "merge
> > commit" option. As a reminder, previous consensus was that we would
> > avoid this since it makes the history harder to follow.
> >
> > Please ensure you are selecting either "rebase commits" or "squash
> > commits" when accepting a PR.
> >
> > I have filed INFRA-18264 to disable the merge commit option.
> >
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Nick Dimiduk <nd...@gmail.com>.
I am +1 for linear commit history.
Does the “squash” option give the committer enough control over the commit
message format and structure? I personally prefer to perform all of my
commit rewriting locally, verify it locally, and then force-push the
desired history to the feature branch before using the “rebase” variant of
the button. Maybe that process is not necessary with the “squash” option?
Thanks,
Nick
On Tue, Apr 23, 2019 at 7:33 AM Sean Busbey <bu...@apache.org> wrote:
> Folks,
>
> Looking at history for the current master branch, we had several PRs
> accepted over the last week where the committer used the "merge
> commit" option. As a reminder, previous consensus was that we would
> avoid this since it makes the history harder to follow.
>
> Please ensure you are selecting either "rebase commits" or "squash
> commits" when accepting a PR.
>
> I have filed INFRA-18264 to disable the merge commit option.
>
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Nikhil Bafna <zo...@gmail.com>.
This can be auto-enforced from Github settings -
https://github.com/apache/hbase/settings
The options under "Merge button" let you select any of
1. Allow merge commits - Add all commits from the head branch to the base
branch with a merge commit.
2. Allow squash merging - Combine all commits from the head branch into a
single commit in the base branch.
3. Allow rebase merging - Add all commits from the head branch onto the
base branch individually.
To enforce, only option 2 should be selected.
-
zodvik
On Wed, Apr 24, 2019 at 12:29 AM Peter Somogyi <ps...@apache.org> wrote:
> Some clarification to my previous email after reading the reply from Sean.
> I meant to squash multiple commits into a single one when the pull request
> touches a single JIRA.
>
> As an example this could be submitted using squash and merge:
> Pull Request: HBASE-XXXX
> Commit 1: HBASE-XXXX
> Commit 2: Additional test case
> Commit 3: Fix checkstyle issue
>
> On Tue, Apr 23, 2019 at 8:37 PM Sean Busbey <bu...@apache.org> wrote:
>
> > Yes. either of the bottom two there would be fine, depending on the PR.
> >
> > If it's all for a single JIRA, I'd personally prefer the middle
> > "Squash and merge" option with an appropriately re-written commit
> > message.
> >
> > If the PR is covering 3 JIRAs together for some reason, then the last
> > option there, "rebase and merge" is what I'd personally prefer.
> >
> > /me shakes fist at overloaded use of "merge" to mean "merge commit"
> > and "accept this PR".
> >
> > On Tue, Apr 23, 2019 at 12:59 PM Stack <st...@duboce.net> wrote:
> > >
> > > Use this link instead
> > >
> >
> https://drive.google.com/file/d/1Rd5tHjEu5XxJaRikcg6oG6MDceJ4GwTs/view?usp=sharing
> > >
> > > On Tue, Apr 23, 2019 at 10:38 AM Stack <st...@duboce.net> wrote:
> > >
> > > > To be clear, you mean the last option here Sean?
> > https://ibb.co/8KrFkBq
> > > > S
> > > >
> > > > On Tue, Apr 23, 2019 at 7:32 AM Sean Busbey <bu...@apache.org>
> wrote:
> > > >
> > > >> Folks,
> > > >>
> > > >> Looking at history for the current master branch, we had several PRs
> > > >> accepted over the last week where the committer used the "merge
> > > >> commit" option. As a reminder, previous consensus was that we would
> > > >> avoid this since it makes the history harder to follow.
> > > >>
> > > >> Please ensure you are selecting either "rebase commits" or "squash
> > > >> commits" when accepting a PR.
> > > >>
> > > >> I have filed INFRA-18264 to disable the merge commit option.
> > > >>
> > > >
> >
>
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Peter Somogyi <ps...@apache.org>.
Some clarification to my previous email after reading the reply from Sean.
I meant to squash multiple commits into a single one when the pull request
touches a single JIRA.
As an example this could be submitted using squash and merge:
Pull Request: HBASE-XXXX
Commit 1: HBASE-XXXX
Commit 2: Additional test case
Commit 3: Fix checkstyle issue
On Tue, Apr 23, 2019 at 8:37 PM Sean Busbey <bu...@apache.org> wrote:
> Yes. either of the bottom two there would be fine, depending on the PR.
>
> If it's all for a single JIRA, I'd personally prefer the middle
> "Squash and merge" option with an appropriately re-written commit
> message.
>
> If the PR is covering 3 JIRAs together for some reason, then the last
> option there, "rebase and merge" is what I'd personally prefer.
>
> /me shakes fist at overloaded use of "merge" to mean "merge commit"
> and "accept this PR".
>
> On Tue, Apr 23, 2019 at 12:59 PM Stack <st...@duboce.net> wrote:
> >
> > Use this link instead
> >
> https://drive.google.com/file/d/1Rd5tHjEu5XxJaRikcg6oG6MDceJ4GwTs/view?usp=sharing
> >
> > On Tue, Apr 23, 2019 at 10:38 AM Stack <st...@duboce.net> wrote:
> >
> > > To be clear, you mean the last option here Sean?
> https://ibb.co/8KrFkBq
> > > S
> > >
> > > On Tue, Apr 23, 2019 at 7:32 AM Sean Busbey <bu...@apache.org> wrote:
> > >
> > >> Folks,
> > >>
> > >> Looking at history for the current master branch, we had several PRs
> > >> accepted over the last week where the committer used the "merge
> > >> commit" option. As a reminder, previous consensus was that we would
> > >> avoid this since it makes the history harder to follow.
> > >>
> > >> Please ensure you are selecting either "rebase commits" or "squash
> > >> commits" when accepting a PR.
> > >>
> > >> I have filed INFRA-18264 to disable the merge commit option.
> > >>
> > >
>
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Sean Busbey <bu...@apache.org>.
Yes. either of the bottom two there would be fine, depending on the PR.
If it's all for a single JIRA, I'd personally prefer the middle
"Squash and merge" option with an appropriately re-written commit
message.
If the PR is covering 3 JIRAs together for some reason, then the last
option there, "rebase and merge" is what I'd personally prefer.
/me shakes fist at overloaded use of "merge" to mean "merge commit"
and "accept this PR".
On Tue, Apr 23, 2019 at 12:59 PM Stack <st...@duboce.net> wrote:
>
> Use this link instead
> https://drive.google.com/file/d/1Rd5tHjEu5XxJaRikcg6oG6MDceJ4GwTs/view?usp=sharing
>
> On Tue, Apr 23, 2019 at 10:38 AM Stack <st...@duboce.net> wrote:
>
> > To be clear, you mean the last option here Sean? https://ibb.co/8KrFkBq
> > S
> >
> > On Tue, Apr 23, 2019 at 7:32 AM Sean Busbey <bu...@apache.org> wrote:
> >
> >> Folks,
> >>
> >> Looking at history for the current master branch, we had several PRs
> >> accepted over the last week where the committer used the "merge
> >> commit" option. As a reminder, previous consensus was that we would
> >> avoid this since it makes the history harder to follow.
> >>
> >> Please ensure you are selecting either "rebase commits" or "squash
> >> commits" when accepting a PR.
> >>
> >> I have filed INFRA-18264 to disable the merge commit option.
> >>
> >
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Stack <st...@duboce.net>.
Use this link instead
https://drive.google.com/file/d/1Rd5tHjEu5XxJaRikcg6oG6MDceJ4GwTs/view?usp=sharing
On Tue, Apr 23, 2019 at 10:38 AM Stack <st...@duboce.net> wrote:
> To be clear, you mean the last option here Sean? https://ibb.co/8KrFkBq
> S
>
> On Tue, Apr 23, 2019 at 7:32 AM Sean Busbey <bu...@apache.org> wrote:
>
>> Folks,
>>
>> Looking at history for the current master branch, we had several PRs
>> accepted over the last week where the committer used the "merge
>> commit" option. As a reminder, previous consensus was that we would
>> avoid this since it makes the history harder to follow.
>>
>> Please ensure you are selecting either "rebase commits" or "squash
>> commits" when accepting a PR.
>>
>> I have filed INFRA-18264 to disable the merge commit option.
>>
>
Re: [DISCUSS] Reminder: please use 'rebase' or 'squash' when
accepting PRs via github
Posted by Stack <st...@duboce.net>.
To be clear, you mean the last option here Sean? https://ibb.co/8KrFkBq
S
On Tue, Apr 23, 2019 at 7:32 AM Sean Busbey <bu...@apache.org> wrote:
> Folks,
>
> Looking at history for the current master branch, we had several PRs
> accepted over the last week where the committer used the "merge
> commit" option. As a reminder, previous consensus was that we would
> avoid this since it makes the history harder to follow.
>
> Please ensure you are selecting either "rebase commits" or "squash
> commits" when accepting a PR.
>
> I have filed INFRA-18264 to disable the merge commit option.
>