You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Editing pull request: Can we rebase instead of merge? #215
-1 from me.
rebase rewrites history.
If we push rewritten history to contributor's branch and the contributor has local change,
the contributor may be surprised when they do git pull.
Since we use "squash and merge" button when merging pull request,
there are no reason to use rebase.
And then add a separate note saying to only use git merge rather than git rebase if you need to bring in changes from the PR's target branch (for the reason @methane describes - we don't want to rewrite the contributor's branch history).
My concern is that when a PR was created quite a long while ago, when we do a merge from a more recent origin\master, it can bring in too many unrelated changes, making it difficult to review perhaps.
But I agree with the point about not wanting to rewrite the contributor's branch history.
I'll leave the instruction as is for now.
Thanks :)
@Mariatta For those cases, it may make sense to ask the contributor if they'd mind doing a rebase before doing a merge ourselves and/or asking them if they're OK with us doing one.
The main problem we want to avoid is doing a rebase without the contributor being aware it's happening.
when a PR was created quite a long while ago, when we do a merge from a more recent origin\master, it can bring in too many unrelated changes, making it difficult to review perhaps.
I think Github won't show commits from master branch.
Github will show only one "merge commit".
Activity
methane commentedon May 31, 2017
-1 from me.
rebase rewrites history.
If we push rewritten history to contributor's branch and the contributor has local change,
the contributor may be surprised when they do
git pull.Since we use "squash and merge" button when merging pull request,
there are no reason to use rebase.
ncoghlan commentedon May 31, 2017
It's possible to skip the merge entirely - that's just the example we used of a change someone might want to make.
It's probably better to simplify the example to:
And then add a separate note saying to only use
git mergerather thangit rebaseif you need to bring in changes from the PR's target branch (for the reason @methane describes - we don't want to rewrite the contributor's branch history).serhiy-storchaka commentedon May 31, 2017
I concur with @methane.
Unfortunately AppVeyor build failed if it can't merge the branch to master without conflicts (which are likely if Misc/NEWS is changed).
Mariatta commentedon May 31, 2017
My concern is that when a PR was created quite a long while ago, when we do a merge from a more recent
origin\master, it can bring in too many unrelated changes, making it difficult to review perhaps.But I agree with the point about not wanting to rewrite the contributor's branch history.
I'll leave the instruction as is for now.
Thanks :)
ncoghlan commentedon Jun 2, 2017
@Mariatta For those cases, it may make sense to ask the contributor if they'd mind doing a rebase before doing a merge ourselves and/or asking them if they're OK with us doing one.
The main problem we want to avoid is doing a rebase without the contributor being aware it's happening.
methane commentedon Jun 2, 2017
I think Github won't show commits from
masterbranch.Github will show only one "merge commit".