Live data from Hacker News

Git rebase, what can go wrong

jvns.ca

351–360 of 404 posts

Re: Git rebase, what can go wrong

#351

Earlier quoted context omitted.

I don't mind merge commits, it's the 100 tiny individual commits some developers seem to like to do that really clutters things up. Yes, I know, git squash is a thing, but not committing until the feature is working and ready to commit is also a thing.

> not committing until the feature is working and ready to commit is also a thing That leaves you prone to losing work if you have a false start that you need to back out of. I prefer to commit early and often on my private branches, then before submitting a pull request I clean up the history to where there are a few good commits that form useful, standalone chunks (ideally the test suite fully passes on each commit…

>That leaves you prone to losing work if you have a false start that you need to back out of.

Hasn't happened to me in over 20 years of using version control. I always keep moving forward, there's really never been a need to go back to a previous commit that hitting crtl-Z wouldn't accomplish just the same. If I wanted to try a new direction I'd just clone the repo again and do the work there. Littering the git history with dozens of superfluous commits just seems pointless. Having to stop and think about writing a commit comment is also just a waste of time - in aggregate it wastes a lot of time. It adds a lot of churn to a workflow for something that may never really be of any value.

Re: Git rebase, what can go wrong

#352

Earlier quoted context omitted.

You can simply pass flags to “git log” to hide merge commits, without needing to rewrite history to “destroy” that information. While they are often noisy, sometimes they can be useful. I usually prefer to hide information rather than destroy it.

I read this justification in nearly every thread that pops up git rebase. I feel like a full because I cannot think of a real world example when this information crosses from signal to noise. Generally, branches that are not ready to merge tend to have enormous amounts of noise commits. Is there a blog post or some concrete examples I could work through that illustrate these benefits? I feel like workflows dramatical…

It's a log of what happened in dev and supports reconstructing history to understand why something worked or didn't in retrospect. "It work when we tried it" "oh this dependency was updated in this merge commit that could have changed the behaviour"

Re: Git rebase, what can go wrong

#353

If I were to write a blog post on this I’d make a few do’s and don’ts (why make a blog post when you can blog in HN comments?) Don’t merge the base branch into a feature branch. Rebase to “update”. Do use rerere and the curse of fixing the same conflict over and over is (almost) gone. Don’t rebase (or force push for other reasons) a shared branch. Rule of thumb here is you can probably rewrite history if you work wit…

> Don’t force push after review, until the review is complete. This keeps the history of the review process but you can later merge the fixups with the commits they logically belong in right before merging. I've had to talk to soooo many developers about this. I want to see what changed since my last review, not restart my review.

When I force push to a branch on GitLab with an open merge request, GitLab retains the previous set of commits, and provides an interface within the merge request to compare the current set of commits to previous sets. I love this feature.

Re: Git rebase, what can go wrong

#354
post #326

Earlier quoted context omitted.

On a long-lived codebase you're going to end up with nearly as many such comments as there are lines of code. Now that's a cluttered mess.

When/how to comment is an art in itself, in no conflict to what I wrote. Either a short quip, doc string, or link to the full story is an accessible combo. Nothing is the correct choice for unsurprising code.

While it's often said that comments should capture the "why" of code, I don't usually think that ought to extend to "cuz ticket#" except when that ticket number is an significant bug/limitation that explains a nasty hack.

Noting each feature ticket that ever affected a line--or even just at the function level--is sort of like maintaining a few thousand incomplete micro-changelogs. Doing it "acceptably well" takes much more effort than grooming the commit history so that someone can click "show change history for selected lines" in their IDE.

Plus consider all the unnecessary noise it makes for people reading the code, or reviewing a PR.

Re: Git rebase, what can go wrong

#355

Earlier quoted context omitted.

I do too, except in cases where it’s being used simply as a more complicated UI for `git merge --squash` and there’s no actual “generate a diff and apply it to a different base commit” going on.

I think we have a rose by any other name situation. I call that a rebase.

That's a badly fitting analogy because there's only one type of flower involved. In this situation, they're saying that most things you might do with "rebase -i" are rebases, except for one.

I'll make a math analogy. Technically a rectangle is a trapezoid, but if someone says tries to draw a distinction between rectangles and proper trapezoids, it's not hard to figure out what they mean.

When rebase -i outputs a single commit, that's a degenerate case. There are statements about rebases that are generally true but not true for that specific kind.

Re: Git rebase, what can go wrong

#356
post #306

Earlier quoted context omitted.

I'm not sure I get you. What do you mean the "latest version of the method"? And why shouldn't code from 18 months ago not work? Some minimal regression testing should be in place for production code, and it is probably also used regularly. So, yes seeing e.g. how "CSV export for class A" is implemented is a great guide for implementing "CSV export for class B".

Most recent. Interfaces change over time. Everything you need is in the most recent copy. Showing an old one invites errors for no benefit.

A commit is not a method, it is a change set potentially affecting many files. Pointing people to the commit used to implement feature A lets them understand the whole story of which components need to change (and how) to implement similar feature B in a way that pointing them to a single method or file doesn't necessarily can.

You would then typically supplement reading the commit with reading the current version of the affected code, but looking at the commit points you in the direction of the files and methods you need to look at.

Re: Git rebase, what can go wrong

#357

Earlier quoted context omitted.

All commits are good points and potentially useful points. Was the bug in the refactoring? In the feature itself? In the resolution of merge conflicts? You can only answer if you don't squash, and it becomes easier to fix the bug if you know the answer.

Sure, but also no one particularly wants to CI every commit inside a PR, so there is a usefulness in `git bisect --first-parent` as the "first pass" of known CI points (merge commits presumably from PRs) to find the "PR that introduced the problem" and then drill down into every smaller commit to see if you can get additional bisect information (from commits that may or may not have passed CI in the first place in de…

I don't do full CI for every commit but I do run the relevant unit tests (or all of them depending on the change and the project) and ensure that they pass.

Re: Git rebase, what can go wrong

#358

Earlier quoted context omitted.

All commits are good points and potentially useful points. Was the bug in the refactoring? In the feature itself? In the resolution of merge conflicts? You can only answer if you don't squash, and it becomes easier to fix the bug if you know the answer.

Sure, but also no one particularly wants to CI every commit inside a PR, so there is a usefulness in `git bisect --first-parent` as the "first pass" of known CI points (merge commits presumably from PRs) to find the "PR that introduced the problem" and then drill down into every smaller commit to see if you can get additional bisect information (from commits that may or may not have passed CI in the first place in de…

I think the point the GP message is making is that, prior to review/merge you extract atomic commits from your WIP that tell a clear, concise story of how the change was made. The reviewer has less built up context so by chunking it like this they can step through each commit one at a time.

IMHO the expectation is that each commit would 100% pass CI, so if you decided to extract some commits and merge that early you can. This is especially useful when a 6 commit PR is reviewed, and the first 3 commits are fine but there is more feedback on the last three. The reviewer can split the first 3 good ones out, get them merged and whittle down the PR to the remaining three. The subsequent follow up will be less.

IME team velocity goes up with this too, and it encourages small and easy to review commits like a Remove to be extracted and merged early.

Since PRs are always as large or larger than commits, I would much rather have a specific commit flagged than have to wade through the whole PR diff. If the PR is not familiar to me, I want to increase my effectiveness narrowing down the cause, so I can fix it faster.

Re: Git rebase, what can go wrong

#359
post #2

I like how Atlassian puts it: > The golden rule of rebasing > Once you understand what rebasing is, the most important thing to learn is when not to do it. The golden rule of git rebase is to never use it on public branches. https://www.atlassian.com/git/tutorials/merging-vs-rebasing#... For me, even though rebasing comes with some trappings, I still greatly prefer it to the alternative, which is to have merge commit…

> I still greatly prefer it to the alternative, which is to have merge commits cluttering up the commit history.

I've heard this many times before, but haven't been able to figure out why this is a problem. In your workflow is it a problem to have a cluttered commit history? If so, could you explain how?

Re: Git rebase, what can go wrong

#360

Earlier quoted context omitted.

If a strategy requires humans to be virtuous AND vigilant it is doomed to failure. Sorry, but I don’t buy that. By the same principle, there’s also no point in writing unit tests or defining static types or having code reviews, all of which require thought and extra work, yet can yield considerable dividends when done even moderately well. I rarely use history and prefer merge/squash, with automated CI tools, and tes…

One of these things is not like the other. (Journey vs. final destination.) As an industry we get paid primarily for 1) working software and 2) communicating with stakeholders. Tidy yet inaccessible (to non-dev) construction stories are not on that path. I would argue unit tests et al are, to ensure #1. No stakeholders? Put why into a readme, where it can be seen at a glance. Comments can reference docs. Complexity m…

Sorry again, but I’m still not seeing the distinction I think you’re trying to make here.

I see version history as an asset, just like the code itself, tests, developer documentation, the bug tracker database… None of these things are directly visible to end users under normal circumstances, but they are useful sources of information and organisation and collaboration that help developers to create the software that users do see.

To me, a repo with a messy version history is like code full of superficial comments, a test suite with high coverage metrics that still doesn’t exercise the most important functionality, a dev team where the only documentation is some auto-generated static site that reproduces what any decent IDE would show in real time anyway, or a tracker where all the tickets are vague one-liners. You can produce useful software despite those things, but why would you?

Post reply on HN