4 ms·

> 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 review

> 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.

pm215 5y ago
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...
- WorldMaker 5y ago
  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.
- raducu 5y ago
  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.
maccard 5y ago
> 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..
- somebodythere 5y ago
  If you squash and merge shouldn't the final pushed code be exactly the same as what was reviewed?
  - WorldMaker 5y ago
    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.

pm215 5y ago

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...

WorldMaker 5y ago
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.
raducu 5y ago
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.

maccard 5y ago

> 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..

somebodythere 5y ago
If you squash and merge shouldn't the final pushed code be exactly the same as what was reviewed?
- WorldMaker 5y ago
  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.