-
-
Notifications
You must be signed in to change notification settings - Fork 4
allow instances that are configured to use rebase to fallback to merges #365
Comments
it looks like the message is now specific about it not being able to rebase (maybe it was always this way, but i think i'd checked in the past and it wasn't this explicit) {
"message":"This branch can't be rebased",
"documentation_url":"https://developer.github.com/v3/pulls/#merge-a-pull-request-merge-button"
} |
@sethsamuel since i prefer rebasing these PRs, i'd love feedback around this from someone with a different merging preference. you mentioned that your team prefers squash merging. are there situations where a PR can be in a state where github prevents a squash merge? with rebasing, this can happen when a conflict is resolved through the web interface, for example. while i havent done it yet, i'm ok with those PRs being merged rather than rebased, so i plan to let greenkeeper-keeper fallback to merges in those scenarios. if that is a realistic situation with squash merges, would you consider such a fallback ok with your conventions, or would you want the fallback to be opt-in? |
My general experience is that so long as you stick to rebases or merges, everything's great, but if you mix the two bad things happen. You can get away with single rebased commits in a merge repo, like the ones GK does in the clean passing cases. The only case I can think of where falling back to merging might cause problems is if the GK branch has commits from master cherry-picked onto it to fix a failing case. I think worst case there is that the fallback will fail and there'll need to be manual intervention. |
i think this is the key point to keep in mind here. if there is a conflict of some sort, github wont allow any form of incorporating the changes. manual intervention is unavoidable in that case. my goal with this change is to keep |
there are some scenarios where rebasing is not possible, but merging would be. in my case, i would prefer a merge over a PR being left open. this could potentially be configurable if some wouldnt want to fallback by default, but i will probably not make it configurable if no one requests that ability.
The text was updated successfully, but these errors were encountered: