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

Option to rebase instead of automatically merge when pulling from upstream #3422

Closed
tbillington opened this issue Nov 27, 2017 · 39 comments · Fixed by #6893
Closed

Option to rebase instead of automatically merge when pulling from upstream #3422

tbillington opened this issue Nov 27, 2017 · 39 comments · Fixed by #6893

Comments

@tbillington
Copy link

Description

Github Desktop automatically merges upstream commits. This is fine for some projects but quickly dirties the git history with unnecessary merges. This has bitten me and colleagues multiple times in this version and the previous version of Github Desktop with the magic "Sync" button.

Proposal

Add an option in the menu to pull rebase instead of merge, or change button text to "Merge" so that I know it's time to go to the terminal so I can pull rebase.

Version

GitHub Desktop version: 1.0.3

OS version: OSX 10.12.6

Note: I did look for similar issues on this topic but couldn't find them.

@PavelDrozdovERM
Copy link

I would also like this feature as old GitHub desktop works that way and it is way nicer

@artyom-kolesnikov
Copy link

We are using GitHub Desktop for artists/designers at work but the history gets dirty very quickly due to lack of this feature :( Will have to switch to more complex git client unfortunately.

@siamkreative
Copy link

👍 Please consider this in your roadmap

@paulballas
Copy link

+1

@j-f1
Copy link
Contributor

j-f1 commented Mar 13, 2018

Please use the 👍 button on the original issue to indicate support.

@iAmWillShepherd
Copy link
Contributor

iAmWillShepherd commented May 17, 2018

Comment from #4711 by @timor-raiman

Rather than a generic rebase, which is a complicated flow to implement in GUI elements, it would be of great use to our team if it were possible to specify globally or per-repository that all 'pull' operations should rather be 'pull -r'.

The part that I want to call attention to is specifying whether to pull -r per repository.

@shiftkey
Copy link
Member

shiftkey commented May 18, 2018

For reference, we added --no-rebase here because we didn't handle merge conflicts well when the user had set pull.rebase=true in their git config: #1627

Improving conflict resolution in the app is on our roadmap and I think is a pre-requisite for this work.

@DHager
Copy link

DHager commented Jun 4, 2018

Like @artyom-kolesnikov , I'm working with some non-developers... Github Desktop's default required merge behavior is proving a bit of a "foot-gun".

@billygriffin billygriffin added the user-research Issues that might benefit from user interviews, validations, and/or usability testing label Jul 30, 2018
@billygriffin
Copy link
Contributor

From #5391:

I often (several times per day) reorder, delete, and squash commits via interactive rebase. It would be nice to implement these commands from the UI client from the History tab by dragging and dropping commits (for reorder and squash) and right click context menu (for delete and rename).

@billygriffin
Copy link
Contributor

From @niik in #1627:

Your global git config forces git pull to use rebase instead of merge and at the moment we don't have any support for rebase (see #1652). Ideally we would respect this setting but I think in the short term our only option is to explicitly tell pull to do a merge rather than a rebase. A stretch goal could be to let the pull do a rebase but if we detect that it fails we could abort it and do a regular merge instead.

@julien-c
Copy link

Hmm, I don't get why this is not a priority 1 at this point. pull -r is the essential command in so many workflows.

@billygriffin
Copy link
Contributor

billygriffin commented Oct 19, 2018

Hi @julien-c, thanks for the feedback. This is something the team is actively working on, and I realized we didn't make that as clear as we could have - that's my fault. Here's our near-term roadmap which has this on there: https://github.com/desktop/desktop/blob/development/docs/process/roadmap.md

I'm also going to create a new issue that lays out the scope of our current work, thanks for the nudge.

@billygriffin billygriffin changed the title Option to rebase instead of automatically merge. Option to rebase instead of automatically merge when pulling from upstream Oct 19, 2018
@masaeedu
Copy link

masaeedu commented Oct 22, 2018

This behavior also causes problems for people that don't use Github Desktop. Many PRs I review keep ending up with commit graph spaghetti, because people still learning the ropes with Git reasonably decide to use a UI tool, and end up implicitly introducing merge commits from the main branch back to their PR branch, without even fully understanding the details of what a merge commit is.

The commit graph view in the Github Desktop UI is not very comprehensive, so it isn't easy to explain what the problem is without resorting to the CLI or a different Git UI (in which case they may as well just learn to use that instead of Github Desktop).

Pestering authors to fix minor details like the commit structure in every other PR results in a very poor impression of Git, when really this is a problem introduced by the "pit of failure" defaults currently enabled in this application.

@ampinsk
Copy link
Contributor

ampinsk commented Oct 31, 2018

Hi everyone,

Thanks for expressing interest in rebasing in GitHub Desktop, and for taking the time to explain your use cases with us. For that, we truly thank you!

This week, we’re exploring the future of rebasing on GitHub Desktop and would love to understand your workflow and get your reaction to some designs we have made.

If you would be open to a 30 minute User Interview with a few of our GitHub Desktop Core team, fill out this form by the end of the day today.

We’re still in our “Research Phase” so any and all thoughts you have are absolutely welcome! If you have any follow up questions for us before filling out the survey, we’d be happy to answer them.

Thank you!

@RoystonS
Copy link

@ampinsk Your user interview team could also consider interviewing those people who specifically do not or cannot currently use the Github Desktop application precisely because it lacks the pull-with-rebase feature. I would be very interested in using the Github Desktop app, but my team and I can't even consider it because of the lack of this feature, so we can't usefully fill in that form or contribute to your research because we're not 'current users'?

@billygriffin
Copy link
Contributor

@RoystonS Thanks so much for your feedback, that's helpful and was an oversight on my part. We updated the form to make explicit that we're open to feedback from both existing and potential future GitHub Desktop users.

@billygriffin billygriffin removed the user-research Issues that might benefit from user interviews, validations, and/or usability testing label Nov 19, 2018
@shiftkey shiftkey added this to the 1.7.0 milestone Jan 7, 2019
@shiftkey
Copy link
Member

shiftkey commented Jan 7, 2019

I'm tentatively scheduling this as in-scope for 1.7.0 because there's an existing pull.rebase config that can be set globally or per-repository, and we plan to undo our workaround in #1760 now that we have a better in-app experience for dealing with conflicts.

Discussions about also having a UI for this will need some input from @desktop/product and @desktop/design to determine when to prioritize this.

@RoystonS
Copy link

RoystonS commented Jan 7, 2019

Btw, a repo-only level config for this (without UI) means I still can't recommend the tool for my team. We usually pull with rebase, but if we have outstanding uncommitted changes, a one-off non-rebase pull (which wouldn't cause a merge) is better. All other tools I've used present a sticky checkbox on a pull dialog.

@shiftkey
Copy link
Member

shiftkey commented Jan 7, 2019

@RoystonS thanks for that extra context about "per-operation" being important rather than config.

I guess this is then more of a UX discussion on top of what's in scope with 1.7. I'll drop this from the milestone in the meantime.

@Croydon
Copy link

Croydon commented Jan 8, 2019

I'm believing that merge commits are polluting the git history so I would like to always use the rebase strategy

@DHager
Copy link

DHager commented Jan 8, 2019

@billygriffin In my case I'm speaking more as a victim of the actual users.

  • The company is trying to get some not-so-technical employees to make actual commits to the repo.
  • It is decided (rightly or wrongly) that the Github Desktop application looks like the perfect official tool for them to make changes to the repo.
  • The users aren't trying to maintain a long-term separate shared branch, they're just trying to save their work periodically and then apply the final changes.

In those circumstances I think rebasing is better, since:

  • The end-users' local history of individual commits is relatively unimportant.
  • It's far easier to explain to them than merging. ("These conflicts show what you were editing, what's there now, and you need to choose how to 'redo' the changes you were working on.")
  • Their use-pattern will result in a lot of merge-commits that don't add value to the history.

@billygriffin
Copy link
Contributor

@DHager Thanks for the perspective. Further discussion is whether we should consider enabling a UI pattern to sometimes use pull merge and sometimes use pull rebase. What I'm hearing from you is that assuming it's set up to do so, you'd always like you and people you work with to use pull --rebase. Is that right? I'm trying to understand how many others share @RoystonS's view that it's helpful to alternate usage patterns between the two depending on the circumstances.

@DHager
Copy link

DHager commented Jan 14, 2019

@billygriffin Yes, I think that asking the user to reformulate their changes (i.e. rebase) is the "safest" default behavior, based on the kinds of users I observed using the tool and their reasons for using it.

@LianH
Copy link

LianH commented Jan 24, 2019

For some added perspective, for me it would be better to have the option to use merge and rebase as specified by me, rather than needing to pick to always use one or the other.

@billygriffin
Copy link
Contributor

Hi @LianH, thanks for sharing! Would you mind describing under what circumstances you'd choose to use each one?

@billygriffin billygriffin added this to the 1.6.3 milestone Feb 4, 2019
@LeeMatthewHiggins
Copy link

Just started using the GitHub app. Having to keep source tree app due to the lack of rebasing. :(

@billygriffin
Copy link
Contributor

Thanks for the feedback @LeeMatthewHiggins, I really appreciate it - we're working on support for pull --rebase currently, and we're hoping to ship explicitly rebasing one branch onto another in Desktop not too long after that.

@shiftkey
Copy link
Member

@billygriffin it's not clear to me where this task sits with respect to the items already in scope for 1.6.3 - are you able to elaborate?

@shiftkey
Copy link
Member

I'm going to drop this from 1.6.3 because we've not had discussions about making this an option in the UI as part of this release. If you have pull.rebase=true set in your Git config you should see the pull button change to show this context:

I think that will go a long way to helping with the pain outlined in this thread!

@billygriffin
Copy link
Contributor

billygriffin commented Feb 27, 2019

After discussing this at length, I'm going to close this issue with #6893 that's shipping in the next beta and very soon to production in 1.6.3. I know this isn't going to perfectly solve everyone's desired use case here, as there's no way to manually select either git pull or git pull --rebase in the UI. However, the behavior we're enabling here does meet the needs of the majority of the people we've heard from to date, and the expectation is straightforward. I'm still trying to reconcile how to meet the needs of folks like @RoystonS and their team, and the recognition that this doesn't quite get us all the way there. So this definitely isn't a final answer, but it's what we're going to ship for now and then evaluate from there.

We're also building on this work to enable explicit rebasing locally with one branch onto another, so that's coming in the not too distant future as well.

To reiterate, we're closing this because we don't want to communicate something we're not necessarily committing to, not because we're unwilling to revisit the question of whether to enable more differentiation in the app and not just rely on the user's git config. Thanks to everyone who weighed in on this, it's really appreciated.

@masaeedu
Copy link

@billygriffin I haven't fully followed the progress of the thread, but does this mean that if I set up people's gitconfigs to have pull.rebase=true, the Github UI will never introduce merge commits from master to their feature branch?

@billygriffin
Copy link
Contributor

billygriffin commented Feb 27, 2019

@masaeedu Yep, local commits will be rebased on top of remote commits if that is set in the user's git config. Our new implementation also introduces a conflict resolution process that hopefully makes that part a bit less painful than otherwise too, but it's still a thing folks can run into (in the same way they can if it's a normal git pull).

@shiftkey
Copy link
Member

shiftkey commented Mar 1, 2019

Btw, a repo-only level config for this (without UI) means I still can't recommend the tool for my team. We usually pull with rebase, but if we have outstanding uncommitted changes, a one-off non-rebase pull (which wouldn't cause a merge) is better. All other tools I've used present a sticky checkbox on a pull dialog.

@RoystonS I want to call out a helpful Git config setting that helps you better manage uncommitted changes in Desktop, without the need to change the behaviour each time you pull:

git config --global rebase.autoStash true.

When set to true, automatically create a temporary stash entry before the operation begins, and apply it after the operation ends. This means that you can run rebase on a dirty worktree. However, use with care: the final stash application after a successful rebase might result in non-trivial conflicts. This option can be overridden by the --no-autostash and --autostash options of git-rebase[1]. Defaults to false. (emphasis mine)

If you have this set to false and have tracked changes, you'll likely be blocked from doing a pull with rebase (git pull by default is fine here):

If there's extra context here about why you think a "non-rebase pull" is better here than leaning on pull.rebase=true globally (I gather you want to avoid creating a merge commit, in which case I'd also ensure pull.ff is set to true) I'm happy to try and incorporate this feedback as part of iterating on this area.

@lilgurl

This comment was marked as spam.

@lilgurl

This comment was marked as spam.

@desktop desktop temporarily blocked lilgurl Oct 31, 2019
@9mm
Copy link

9mm commented Apr 17, 2020

This thread has been helpful. @shiftkey do you know by chance if your message there matches the old Desktop app for OSX? The flow on that was literally perfect, and I'm trying to recreate that on version 2. I just don't know the nuances of stash and whatnot, based on how the old app used to work.

@shiftkey
Copy link
Member

@9mm the stashing flow of the classic app is very different to how it works in the current app. I wrote a while ago about the reasons why it didn't come over as-is #1633 (comment).

You should look over #6107 which outlined the approach for stashing in the current app.

@9mm
Copy link

9mm commented Apr 17, 2020

@shiftkey thanks!

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

Successfully merging a pull request may close this issue.