4 ms·

I think most people these days just look at PRs. Everything else is largely noise.

by osigurdson 2y ago

saagarjha 2y ago
Not at all. Someone’s got to look at your commits in the future when your code breaks ;)
- deleted 2y ago
  [deleted]
- Izkata 2y ago
  Yep, this is why I'm mostly against squashing (and completely against blind squash-merges).
  - Spivak 2y ago
    I'm not sure I get the advantage. The only thing I know is that the last commit on each PR is the one that is claimed to work. All others might as well be noise at that point since those random intermediates were never HEAD on the main branch, might be broken, incomplete, have failing tests, etc.. Squashing every PR into a single commit is at least an honest history of what's actually going out. If you squash you have a history where every commit was tested and works (bugs notwithstanding) which to me is way more useful.
    - saagarjha 2y ago
      I mean you should be designing your commits such that each individual commit builds. That's the point of using squashes to fix up your history!
      - osigurdson 2y ago
        Commit refactoring can be really hard work however. Basically you do something like taking N random commits and convert these into M logical ones - where each one delivers incremental value and builds upon the other. For some types of work it is easy, N=M: you were able to do high quality value adding atomic commits for the whole PR without rework. For other types N >> M. This can happen when trying different approaches to a hard problem. I suppose research type work could always be considered a POC and the actual implementation could be a kind of cleanroom re-implementation of the POC but there isn't always time for such things and (again) the PR is far more important than the commits that built up to it - particularly if the resultant code is of equal quality. Note that I am not advocating for long running branches here - trunk based development is generally better provided it doesn't over incentivize teams to avoid hard problems (but that is a topic for another day). This is why I think git should include the PR as a first class concept. For simple N=M type work, 1 PR should generally be 1 commit. Why not, after all, make the PR small and easy to review when you can? For harder N >> M type work, you get one PR with many commits that one can dig into if necessary.
    - Izkata 2y ago
      > (bugs notwithstanding) This is the reason. I've been on a maintenance team for years where almost everything we handle was written by people no longer at the company, and often enough I've seen bugs get introduced during the original work, where the fix ends up being obvious because I can see the original commits and how the code got into its current state. A squash of any sort would've hidden the refactor and made it much more difficult. My favorite are ones where "linting" and code formatter commits introduce bugs. Keep those separate from your actual work, please.
  - saagarjha 2y ago
    Oh I'm for squashing to make the history make sense. Please do not blindly squash-merge though.
- watwut 2y ago
  And messy intermediate states won't help him at all. It won't help him either when related commits are interwinned with unrelated commits in history rather then being together.

I think most people these days just look at PRs. Everything else is largely noise.

saagarjha 2y ago

Not at all. Someone’s got to look at your commits in the future when your code breaks ;)

deleted 2y ago
[deleted]
Izkata 2y ago
Yep, this is why I'm mostly against squashing (and completely against blind squash-merges).
- Spivak 2y ago
  I'm not sure I get the advantage. The only thing I know is that the last commit on each PR is the one that is claimed to work. All others might as well be noise at that point since those random intermediates were never HEAD on the main branch, might be broken, incomplete, have failing tests, etc.. Squashing every PR into a single commit is at least an honest history of what's actually going out. If you squash you have a history where every commit was tested and works (bugs notwithstanding) which to me is way more useful.
  - saagarjha 2y ago
    I mean you should be designing your commits such that each individual commit builds. That's the point of using squashes to fix up your history!
    - osigurdson 2y ago
      Commit refactoring can be really hard work however. Basically you do something like taking N random commits and convert these into M logical ones - where each one delivers incremental value and builds upon the other. For some types of work it is easy, N=M: you were able to do high quality value adding atomic commits for the whole PR without rework. For other types N >> M. This can happen when trying different approaches to a hard problem. I suppose research type work could always be considered a POC and the actual implementation could be a kind of cleanroom re-implementation of the POC but there isn't always time for such things and (again) the PR is far more important than the commits that built up to it - particularly if the resultant code is of equal quality. Note that I am not advocating for long running branches here - trunk based development is generally better provided it doesn't over incentivize teams to avoid hard problems (but that is a topic for another day). This is why I think git should include the PR as a first class concept. For simple N=M type work, 1 PR should generally be 1 commit. Why not, after all, make the PR small and easy to review when you can? For harder N >> M type work, you get one PR with many commits that one can dig into if necessary.
  - Izkata 2y ago
    > (bugs notwithstanding) This is the reason. I've been on a maintenance team for years where almost everything we handle was written by people no longer at the company, and often enough I've seen bugs get introduced during the original work, where the fix ends up being obvious because I can see the original commits and how the code got into its current state. A squash of any sort would've hidden the refactor and made it much more difficult. My favorite are ones where "linting" and code formatter commits introduce bugs. Keep those separate from your actual work, please.
- saagarjha 2y ago
  Oh I'm for squashing to make the history make sense. Please do not blindly squash-merge though.
watwut 2y ago
And messy intermediate states won't help him at all. It won't help him either when related commits are interwinned with unrelated commits in history rather then being together.