Skip to content
This repository has been archived by the owner on Sep 6, 2020. It is now read-only.

What if I want the autorebase feature but not the automerge ? #41

Closed
DavG opened this issue Feb 27, 2019 · 11 comments
Closed

What if I want the autorebase feature but not the automerge ? #41

DavG opened this issue Feb 27, 2019 · 11 comments
Assignees
Labels
enhancement New feature or request

Comments

@DavG
Copy link

DavG commented Feb 27, 2019

Hello,

Autorebase looks verycool, but when we merge we deploy, so i don't want merging to be done by this tool.

How can I use only the autorebase feature ?

@leonardfactory
Copy link

leonardfactory commented Mar 8, 2019

I'm interested in this too. @tibdex Are you interested in a PR introducing this feature? Maybe – since label would not be enough – adding comment commands (i.e. LGTM or /autorebase)? Awesome work anyway! 🎉

@tibdex
Copy link
Owner

tibdex commented Mar 8, 2019

Hello guys,

I’ve been hesitating over adding the one-time /autorebase command since I created this project. Until now, I always decided against it because triggering many manual rebases would IMO be a bad practice.

Indeed, rebasing rewrites the history which makes collaborating on pull requests riskier and code reviews unpleasant because of the increased difficulty to keep track of the changes. That’s why it’s currently recommended to use the autorebase label only after a pull request is green and approved, meaning that most of the work on it is done.

On the other hand, manual rebases are sometimes handy. A use case is when the CI config changed on the base branch and you need to rebase on top of it to have a chance to merge your PR. Another one would be to incorporate a feature that just landed on the base branch to see if it plays well with the current PR.

If you could detail your use cases it would help me decide whether on-demand rebase is a good idea or not.

@leonardfactory
Copy link

Hello guys,

I’ve been hesitating over adding the one-time /autorebase command since I created this project. Until now, I always decided against it because triggering many manual rebases would IMO be a bad practice.

Indeed, rebasing rewrites the history which makes collaborating on pull requests riskier and code reviews unpleasant because of the increased difficulty to keep track of the changes. That’s why it’s currently recommended to use the autorebase label only after a pull request is green and approved, meaning that most of the work on it is done.

I agree with you, rebasing many times is something I want to avoid.

On the other hand, manual rebases are sometimes handy. A use case is when the CI config changed on the base branch and you need to rebase on top of it to have a chance to merge your PR. Another one would be to incorporate a feature that just landed on the base branch to see if it plays well with the current PR.

If you could detail your use cases it would help me decide whether on-demand rebase is a good idea or not.

In my thoughts, I'm seeing two "stages":

  1. Development on feature
  2. Feature integration and deployment

Step (1) should be done before rebasing, so that no one could add more commits once it has been rebased. This enforces the team to check code, review it before merging. Step (2) means: "Ok, the feature is ready, now we would like to ship it when stakeholders say so".

Especially if you've got auto-deployment, but even with manual deploys always from master, you would like to distinguish between these two steps. And the step 2 may also include testing the PR against new base, etc.

However, in order to do this, you'll need to rebase against default branch once more, but since this stage (n.2) is conceptually isolated from development, rebased branch would flow into master only once. So history rewrite could be acceptable.

This is my idea right now, and in this context an /autorebase seems useful, even if I understand your thoughts (if someone starts using the command in active development it would be a mess in no time).

I would like however to know your point, since maybe it could be solved using another flow that I didn't think of.

@asbjornu
Copy link

I'd like to be able to autorebase without merge as well. I want to merge manually with GitHub's UI, but since GitHub doesn't provide a way to rebase a pull request on top of HEAD of the target branch, autorebase would have been very handy.

Asking pull request contributors to do the following in each and every pull request to avoid merge commits from the target branch (introduced when clicking the "Update branch" button in the GitHub UI) is just too much of a burden:

git remote add upstream <url>
git fetch upstream
git checkout <pull-request-branch>
git rebase upstream/<target-branch>
git push --force

So being able to just autorebase the PR without it being merged would be much easier. Even asking the PR contributor to do git fetch origin && git checkout <pull-request-branch> && git reset origin/<pull-request-branch> would be much easier than the tedious task of having to add upstream and such described above.

Please make it possible to autorebase without merge.

@tibdex
Copy link
Owner

tibdex commented Mar 23, 2019

Hi guys, I've been working on https://github.com/tibdex/backport lately but this issue is the next thing I want to work on. Do you agree that this is the feature you need:

As a user with write permission, I can post an /autorebase comment on a pull request to rebase it once.

?

@leonardfactory
Copy link

Hi @tibdex! Something like that seems fine to me. The write permission is indeed needed 👍

@tibdex tibdex self-assigned this Mar 23, 2019
@tibdex tibdex added the enhancement New feature or request label Mar 23, 2019
@asbjornu
Copy link

That sounds perfect, @tibdex!

@tibdex
Copy link
Owner

tibdex commented Mar 31, 2019

Feature added!

@tibdex tibdex closed this as completed Mar 31, 2019
@thedrow
Copy link

thedrow commented Jun 8, 2019

Is this deployed?
I installed the app for Celery and no rebase occurred.

@tibdex
Copy link
Owner

tibdex commented Jun 8, 2019

@thedrow your comment needs to contain only /rebase, nothing else.

@thedrow
Copy link

thedrow commented Jun 10, 2019

@tibdex I think the documentation should say that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants