Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I have to disagree. Mainly because I care what the contents of the branch were after the merge when I have to go back and figure out why a particular error is occurring.

Mainly my workflow is:

1. Commit often with moderately useful commit titles/texts.

2. Rebase -i prior to pushing to remote. During rebase, squash/fixup together relevant commits and write succinct commit titles with useful commit texts. Then push.

3. Rebase -i again at the end of the PR to guarantee everything is coherent and that each commit that makes it in the merge at least builds and passes existing tests (i.e. that functionality before the branch isn't broken in the branch).

4. Merge into master. Merge commit should include the PR info (like PR text contents) in the merge text if at all possible.

5. Forget. Maybe move to a new git host.

The years later when I identify a bug I can just:

1. Put together a reproducible example.

2. Git bisect --first-parent (only top level commits/merges) to identify the merge where the bug was introduced.

3. Git bisect the merge itself to identify the exact commit that introduced it.

4. From there I can determine the context of hows and whys behind the bug being introduced, identify a possible solution, and evaluate any process changes that could have prevented this bug from being introduced elsewhere in the project.



Step 3 is problematic. When you push change requests you also need to rebase i.e. put the fixing commits right after the affected commits.

Otherwise, if you only rebase once the PR is complete, then you are likely to stumble upon conflicts due to the order of affected files.

It's a waste of the developer's time either way, not to mention that rebasing i.e. changing history on an (upstream) PR hurts the review experience e.g. comments on lost commits.


I generally agree that it's not ideal.

When I say rebasing, mostly I mean going back to clean up commit texts/titles and occassionally to squash/fixup commits that at the end of the dev process don't really make sense anymore and would probably only confuse people looking back at it (like squashing/fixuping down "do a, do b, revert a, do c, revert b, do d" into "do c, do d").

I make the distinction from step 2 because while I forgot to mention it (and in my head was considering it a part of step 3), I'll generally rebase right before I open the PR which cleans up most of the noise (such as "merge master even though no relevant files were touched" commits which don't add anything meaningful). Then the rebase at the end of the PR is only really squashing down a lot of the back and forth on the code review into meaningful chunks.

On Github this does kinda mess with the code snippet/suggestion stuff but since it's right before merge, all of those should be closed as resolved/implemented/etc anyways.

As for it being a waste of time or not, I'd disagree. I spend maybe 5 minutes at absolute most rebasing and often only about a minute (really just git rebase -i, mark the relevant commits e/s/f, let it do it's thing poking it where necessary, resigning w/ gpg, and pushing). If the rebase is throwing nasty conflicts while working on fairly linear single-dev feature branches, odds are you are rebasing wrong. And I find this saves future me a lot of time since things generally "just work" and I have all the important history still in git.


> changing history on an (upstream) PR hurts the review experience e.g. comments on lost commits.

It does, but this is specifically a weakness of GitHub's UI, not of the process itself. It's handled better by other tools.


> rebasing i.e. changing history on an (upstream) PR hurts the review experience e.g. comments on lost commits.

GitLab handles it well.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: