Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] pull-request:merge --squash should perform a rebase against base branch #633

Open
phansys opened this issue Aug 26, 2016 · 6 comments
Labels

Comments

@phansys
Copy link
Member

phansys commented Aug 26, 2016

pull-request:merge --squash should perform a rebase against base branch. I've used this command and it sometimes result in regressions for the base branch, since the squashed commits don't take into account the new commits (those that was created after creating the branch and before the merge) at the base branch.

@cordoval
Copy link
Member

The rebase is not automatic, it should be done manual because in doing it there could be many conflicts.

@phansys
Copy link
Member Author

phansys commented Aug 27, 2016

In my experience, it must be automatic, otherwise the squashed commits sometimes reverts "silently" changes done at base branch after the PR creation but before its merge, since GH diff isn't showing that situation. So, in short words, a conflicted rebase could warn about issues between the histories that the command is trying to merge, and if it happens, the PR can be marked as not ready to be merged.

@cordoval
Copy link
Member

what happens if it fails the rebase? or requires intervention? as often they do. It should just give a warning about, please make sure it is rebased, or the command only should work when it ensures the PR is rebased. Otherwise gives a note, i think git has a way to find out quickly.

@phansys
Copy link
Member Author

phansys commented Aug 27, 2016

Sure, that's the main idea, try to rebase automatically, abort the rebase if it has failed, warn the user about the situation and aborting the PR merge operation. But IMO it is much more painless than let merge PRs that reverts previous changes without any notice of that situation. Last week I found myself reverting PRs for this reason, that only happen with the use of --squash option in PRs with more than one commit as I mentioned before.

@cordoval
Copy link
Member

what i mean is there is a way to check already if it is rebased or not via git. That is all, so we can just do an early check for that and abort or even prevent from running the command very early. I personally prefer to do things observing, so i don't often let gush do my rebases, this is in my experience, because i know there are always things I want to do besides skipping any errors. I honestly think the squash thing is only useful in FOSS context, but i do not use squash at work.

@phansys
Copy link
Member Author

phansys commented Aug 27, 2016

I tend to work the same way at proprietary / FOSS environments, but I've updated the PR taking into account your concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants