Hacker News new | ask | show | jobs
by eertami 1735 days ago
> because I care about the convenience of the person that is actually interested in investigating a feature/bug 5 years from now and not of the lazy code reviewer now

Doesn't seem like such a big problem to make a PR with multiple commits, and then squash and merge into 1 commit after the review.

2 comments

Personally I think that is pessimal, because if I am the person investigating in 5 years time I want to see the change split up into multiple logical commits. So lots-of-commits-plus-squash-and-merge requires the developer to do the work of splitting up the feature implementation but doesn't give the benefit of having a usefully split set of commits to the future-debugging-person...
The change is a feature, it is by deffinition a "logical commit".

Like "migrated this JMS queue to kafka", that can span across tens of files.

Just because the commit is big doesn't mean it's not also logical and consistent.

In fact the reverse can be said -- if you split the commit in smaller commits (and your build is still green with each commit) -- each commit can seem contrieved, and good luck later on, when you try to understand how a piece of code works and how this xml/drl/properties file is connected to this piece of code if you have multiple commits.

If it's a single commit, you can usually narrow down your search tremendously and can easily see the logical deoendencies.

So for me, if it's a JIRA issue, I usually squash my commits before I push.

I agree. If you want the "clean" straight line from the git DAG just use --no-ff in your PR system and you get a nice clean integration log with things like git log --first-parent (git praise --first-parent, git bisect --first-parent) and can still keep the extra history around for those of us that find it useful when investigating things 5/10 years down the road.
> Doesn't seem like such a big problem to make a PR with multiple commits, and then squash and merge into 1 commit after the review.

I fundamentally disagree here. If you're reviewing code, the code that is pushed should be what is reviewed, not a draft of what is reviewed..

If you squash and merge shouldn't the final pushed code be exactly the same as what was reviewed?
Not necessarily. There's a lot of dark arts to merges and there's a lot that can be hidden in them. If you've never heard of, as just one for instance, the git rerere cache and campfire horror stories of how it can be corrupted consider yourself lucky.