Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Git rebase, what can go wrong

jvns.ca

Post reply on HN
Print view View original

81–90 of 404 posts

Re: Git rebase, what can go wrong

#81

Post by gray_-_wolf »

diek wrote: post #63
nerdponx wrote: post #16

Earlier quoted context omitted.

This is the big one for me. destroying Commit information just to keep the graph tidy is a bad idea in my opinion. It would be better if Git provided better tools for filtering the log, e.g. providing some mechanism to elide commits from parents of any merge commit other than the 1st.

> destroying Commit information just to keep the graph tidy is a bad idea in my opinion The commit information I see when telling teams to squash their branches on merge is not valuable. * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test" (note, the broken tests were broken by the changes in the PR) As soon as that PR is merged those commits are worthless. And there are…

> * "fixing whitespace" * "incorporate review comments" * "fix broken test" * "fix other broken test"

Things like this should not be standalone commits though, they should be incorporated into the previous branch by amending the original work. It takes some effort to have a useful git history, it does not just happen on its own.

Re: Git rebase, what can go wrong

#82

Post by ravenstine »

> force pushing makes code reviews harder

On any code base I've worked on that's larger than a small FOSS project, I've found that this simply isn't avoidable. Yes, there's merge commits but, for reasons I won't go into, I think those are worse than the alternative of rebasing and making code reviews difficult.

> One way to avoid this is to push new commits addressing the review comments, and then after the PR is approved do a rebase to reorganize everything.

Not realistic when working on a code base where PRs are being squash-merged every hour and the code review lasts for days.

The best middle-ground is to avoid rebasing until the current wave of feedback has been resolved, even if no one has actually approved yet.

Re: Git rebase, what can go wrong

#83

Post by heisenberg302 »

gus_massa wrote: post #65

> undoing a rebase is hard I do a lot of rebases, but they are trivial rebases in a feature branch, like changing the order of new features and fixups, and then squashing the fixups to make a nice PR. Don't try to do smart weird rebases!!! From time to time I have to make a smart weird rebase because my small commits order is just a mess, or I have to split a commit or something unusual. The important first step is t…

also you can use git rerere if you need multiple goes for the rebase, it saves your conflict resolutions and applies them automatically when they occur again(you need to resolve the conflict when it happens for the first time).

Re: Git rebase, what can go wrong

#84

Post by qhwudbebd »

ajkjk wrote: post #8

I love rebase (I'm a tip-of-master-only person, no merges ever, squash all your commits with `rebase -i` before pushing and write one good commit message for the group). But there's one really, really irritating thing about them: You should not be able to use `--amend` during a rebase. For me editing all my changes onto the commit I'm working on with `git commit -a --amend` (or as I've aliased it, `gcaa`) is automati…

Random thought: given you already have the gcaa alias, perhaps you could include a check that .git/REBASE_HEAD doesn't exist in that?

Probably easiest as a little shell function like

    gcca() {
      local GIT_DIR
      if ! GIT_DIR=$(git rev-parse --git-dir); then
        return 1
      elif test -f "$GIT_DIR/REBASE_HEAD"; then
        printf 'Rebase in progress: commit --amend is disabled\n' >&2
        return 1
      fi
      git commit -a --amend "$@"
    }
rather than an alias?

[Edit] I forgot about rev-parse --verify, which simplifies this further:

    gcca() {
      if git rev-parse --verify REBASE_HEAD >/dev/null 2>&1; then
        printf 'Rebase in progress: commit --amend is disabled\n' >&2
        return 1
      fi
      git commit -a --amend "$@"
    }
This also leaves you still able to use commit --amend long-hand if (for example) you want to edit one of your own commits during rebase -i.

Re: Git rebase, what can go wrong

#85

Post by mvdtnz »

johnsbrayton wrote: post #37
mablopoule wrote: post #13

Earlier quoted context omitted.

I actually hate squash merge because of all the noise it adds. Sure, the commit graph looks nicer, but it come with a terrible loss of information when doing git blame. I'm a big proponent of rebase and squash if it helps to make a commit more coherent, but we use squash merges by default in the current project I'm working on, and I die a little bit each time I try to understand what changes were related to a line wh…

I do squash merges but keep the feature branches. So after determining that I made a change as part of a big pull request, I can then look at the commit/blame history for the pull request source branch if necessary.

I'm guessing you don't work on large projects. This would create an outrageous amount of noise in a busy repository.

Re: Git rebase, what can go wrong

#86

Post by richbell »

spankalee wrote: post #10
richbell wrote: post #6

Earlier quoted context omitted.

> I still greatly prefer it to the alternative, which is to have merge commits cluttering up the commit history. GitHub recently added a feature that prompts people to update their branches via merge. It's frustrating because every PR now had dozens of merge commits polluting the history.

A PR with merges is fine by me, it lets me see how the PR has evolved. What I want is for GitHub to track changes between sets of commits in a PR so that you can do most of the review with merges and "address review comments" commits, and then rebase into well organized, logical commits and review that those have the same diff as the messy history after a force push.

The problem is PRs that have It wouldn't be a problem if people took the time to organize the history prior to merging as you said, but most people don't do this.

Re: Git rebase, what can go wrong

#87

Post by FeepingCreature »

sidlls wrote: post #54
dahart wrote: post #42

Earlier quoted context omitted.

What I think I see these days is squash merges being used lazily to avoid having to do anything to build a clean history with clearly semantically delineated commits. Squash merges are good compared to an alternative where people check in super messy noisy branches, but they unfortunately have a big downside because squash merges can make bisecting and history spelunking more difficult, when the branches that are squ…

What is a "semantically delineated commit"? What is a "clean history"? Why are these two things important?

Not parent: there are technical commits, such as "fix review", "fix jenkins", "fix typo" etc. Those don't delineate a particular feature but a fix for a problem that arose from the workflow. This ends up with a history of "big feature commit that is wrong in three trivial ways" + "fix 1" + "fix 2" + "fix 3". Of those, "big feature commit" is the important one, but "fix 3" is the only working one. This is clearly silly; you should pretend you were perfect from the start and squash "fix 1" through "fix 3" into "big feature commit". Your typos and brainfarts are not of historical relevance.

Re: Git rebase, what can go wrong

#88

Post by __MatrixMan__ »

AndrewDucker wrote: post #70

I find it fascinating that people talk about "Having a history of what people did" in such emotive terms - "Cluttering", "Polluting". What matters is that you end up with working systems. That a lot of change happened is just, well, what happened. It doesn't need to be prettied up and made to look like your development occurred in a clockwork march of cleanliness. It literally does not matter unless you spend a lot o…

What's worse, the desire for cleanliness ends up making things like `git bisect` less useful.

If I had a bad day and introduced something stupid, I want a bisect to point me a the code I wrote on that bad day. If you squash liberally, perhaps because you want each commit to correspond with a release-note, you're going to lose that debugging granulariry.

Re: Git rebase, what can go wrong

#89

Post by ender341341 »

lacrosse_tannin wrote: post #50
sdfghswe wrote: post #33

Just don't rebase something that you've shared with others. It's just rude.

Pull requests after comments and revision?

Doing a squish merge on the actual merge, at least on github doing a rebase mid-pr makes the PR act funny (comments point at non-existent revisions and such).

When I was at amazon their internal PR tool handled them just fine so I would do them in that case.

There's more nuance to the OPs comment along the lines of

> Don't rebase on branches others are working on

On a pr branch I usually at most expect others to pull and do a build to run it locally so I'm not very worried about wiping out changes.

Re: Git rebase, what can go wrong

#90

Post by FeepingCreature »

AndrewDucker wrote: post #70

I find it fascinating that people talk about "Having a history of what people did" in such emotive terms - "Cluttering", "Polluting". What matters is that you end up with working systems. That a lot of change happened is just, well, what happened. It doesn't need to be prettied up and made to look like your development occurred in a clockwork march of cleanliness. It literally does not matter unless you spend a lot o…

> What matters is that you end up with working systems. That a lot of change happened is just, well, what happened. It doesn't need to be prettied up and made to look like your development occurred in a clockwork march of cleanliness. It literally does not matter unless you spend a lot of time doing git-bisect.

And git blame. And git checkout to a past state. It "doesn't matter" only if ease of understanding your project history doesn't matter.

Post reply on HN