7 ms·

Is there anyone else that enforces a simple "just squash & merge everything from PRs into main" across the entire team? I'm comfortable git fooing w/e is neces

Is there anyone else that enforces a simple "just squash & merge everything from PRs into main" across the entire team? I'm comfortable git fooing w/e is necessary, but ever since we adopted this, git related conversations went to almost zero. It's great.

keybored 2y ago
> Is there anyone else that enforces a simple "just squash & merge everything from PRs into main" across the entire team? No. This kind of policy has plenty of downsides[1] and the upsides are relatively minor. [1] https://news.ycombinator.com/item?id=41653892 https://news.ycombinator.com/item?id=41653892
- skydhash 2y ago
  The upsides are great. PRs should be atomic, introducing changes as a set. I don’t care about the history of the branch, I only care that the changes in the PR are one unit that can be reverted as a whole. And each commit in the main branch passes the tests.
  - keybored 2y ago
    I get those so-called upsides with `git log --first-parent`. I can view the changes as one set of changes or all of the individual ones. But I can’t relate in general to those workflows where PRs have to be tiny and revertable (why need to revert so often).
    - skydhash 2y ago
      It’s mostly about project management. In a project, the unit is a task. These tasks will then have discussion around then and then get assigned to the people that are going to do the implementation. After the latter is done and approved, it will be deployed and the task will be marked as done. Having your git history reflecting this is great when you need to take decisions. Because no one will care about your individual commits. Every discussion will revolve about the task. So you make the PRs a bijective relation with it, and by extension the commits on the main branch. Then for special group of tasks like features, you have a specific branch you manage the same way. The other branches you create PRs from can be as messy as you like. And your local copy is yours to do whatever you want.
      - keybored 2y ago
        I know what project management and tasks are. Commits are in practice a superset of all of that. Some commits will correspond to a task. But many commits will just do the work without having to go through the beaurocracy of “project management”. Having to “project manage” each and every commit if you want a useful and fine-grained (but not micromanaged) version control history is impractical. There’s a lot of small tasks in the concrete code. It doesn’t make sense to give every little change a task. And the flipside of that is that you can give up on making commits that are fine-grained enough. Now you’re back in squash town where, sure, your task list is just right. But your commits are sometimes a jumble of different concerns in order to stick to one-task-on-commit. > Because no one will care about your individual commits. Will someone care about your tasks? You don’t pre-empt what commits you care about when you are trying to find the reason for some change in the Git history. You don’t find the exact commit that explains it, clear as day, and then throw it away because there is no associated task. > So you make the PRs a bijective relation with it, and by extension the commits on the main branch. It sounds useful with a mathematical name I guess? Insisting on one-to-one mappings for such small things like commits smells of micromanagement. Yes, either people-management or your own time management.
        skydhash 2y ago
        > Having to “project manage” each and every commit if you want a useful and fine-grained (but not micromanaged) version control history is impractical. You're misunderstand me there. I'm talking about the main branch's commits, on the main repository, not every branch and every commits, and not the local repos. And it's not dogmatic, just very nice to have. If you want a hotfix, it's very easy to create a new branch, commit the fix, push it, create a PR, and squash merge it. Unless you like to edit "main" directly on a collaborative project. Or you can then instruct everyone how to cleanup their commits and make sure that each one is atomic. And you can merge your usual way. I have no dog in this fight. It's just that the above is easier to do. It's pretty much the same result.
        keybored 2y ago
        > You're misunderstand me there. I'm talking about the main branch's commits, on the main repository, not every branch and every commits, and not the local repos. No I don’t. I’m also talking about the commits that go into the main branch. The “permanent history”, not all the commits that happen on feature branches. > And it's not dogmatic, just very nice to have. A squash-only policy is by definition dogmatic. That’s the only thing that I’ve argued against here—a policy. Having the freedom to do it or not is totally different. > Or you can then instruct everyone how to cleanup their commits and make sure that each one is atomic. And you can merge your usual way. I have no dog in this fight. It's just that the above is easier to do. It's pretty much the same result. No. OR you can let people do what they want. Let them squash if they want. If squashing is such a win they can do that. No instructions necessary.
Chromozon 2y ago
My job follows this, and "rebase" isn't even in the vocabulary at work. It's just so simple- our developers spend zero time with git issues because there aren't any. The master branch history ends up being a sequence of pull requests that all have a corresponding code review. Everything stays nice and tidy.
- keybored 2y ago
  Rebase isn’t in our vocabulary either. Because those of us who use it just do it. And those that don’t, don’t. And so there is no reason to interfere with anyone’s way of working.
Degorath 2y ago
But that way you make stacked diffs/changes/PRs really hard, so is it really worth it? I firmly believe GitHub makes reviewing individual commits in a PR so painful that I'd rather not do that.
dbalatero 2y ago
Yep, IMO this is always the best of the 3 strategies. The PR is the unit of work, people get too hung up on the PR's individual commits. Worst: rebase and merge - you end up with your coworker's broken WIP commits all over master, and have a terrible git revert story OK: merge commit - you can revert, but there is a less intuitive `-m 1` flag (IIRC?) you have to pass into revert, and IMO you rarely need the intermediate history. Best: squash & merge - you get one single commit representing the unit of work merging in, git revert is dead easy Also setting the commit message in main to the `{title} (#{prNum})\n\n{prDescription}` format preserves all the good context from your PR and lets you get back to it if you need.
- keybored 2y ago
  Three options? The best alternative to squash-merge is to freely rebase before you merge. Or for that matter squash if you just end up with one commit (because rebase subsumes squash). 1. You get rid of WIP commits, typo fix commits, all kinds of transient changes (for review and so on) 2. You keep the substantive ones 3. All changes are logically separated I don’t see where you cover this option. > Also setting the commit message in main to the `{title} (#{prNum})\n\n{prDescription}` format preserves all the good context from your PR and lets you get back to it if you need. Again. It is ironic that people are so comfortable with dumping the history in a webapp when you are working with a version control system. I am fine with a lot of the history being on GitHub or whatever other webapp. The review history, that is. But I’m not comfortable with leaving the history of the pre-squashed commits on GitHub if those pre-squashed commits were useful for the long-term history.
  - kccqzy 2y ago
    If the code author deems the pre-squashed commits useful, they would've split their PR into multiple PRs. I personally do this all the time: each PR has only one commit, but PRs can depend on each other. And if the PR author deems their commits insignificant, they can feel free to make one PR and they squash & merge.
    - keybored 2y ago
kccqzy 2y ago
Yes after I recently discovered that even with squash & merge, the intermediate history isn't lost: they are not in the git history but they can still be viewed on GitHub.
- mkesper 2y ago
  But then git isn't your source of truth anymore. GitHub still references these commits until they're garbage collected one day.
  - kccqzy 2y ago
    My understanding is that these commits don't get garbage collected ever. After all they still need to support `gh pr checkout` so they need to store them forever as part of the GitHub repo. Sure they won't be included in `git clone` but that's kind of the point.
    - keybored 2y ago
      That’s correct. They will not ge GCd. And yet that’s not usually what the problem is with having your canonical history “over there” in a proprietary webapp. But if the history is easily available on those PR refs then I guess it is (under doubt) fine.
      - skydhash 2y ago
        If you really care, you can use the API to extract the PR’s data in a local archive.
        keybored 2y ago
        If I really care about the version control history? Yeah. Alternatively I could stick with git(1) which does all of this stuff out of the box. Instead of having to learn a superflouous (for the job) API.
zabil 2y ago
We do this. We deploy on commit to main and use the commit SHA for versioning. So squash merges makes it easier to know which commit caused a bug or bisect. This work well for our PR based workflows. If required the PR can be used as reference for un-squashed commits.
- keybored 2y ago
  If you deploy on merge (squash or regular) to main then having the commits squashed makes no difference with regards to finding out what was deployed.
deleted 2y ago
[deleted]
AnthOlei 2y ago
I’ve recently spent time truly learning git, and I’m realizing how much better it is to take care of your commits in a PR even if you squash to main. My reviewers love when I write “review commit by commit” in the PR description. Then each individual commit has its own reasoning, and you can mentally switch into reviewing if that commit does it’s one thing it’s supposed to do correctly. I will accept the argument that each commit should be its own PR though :)
- keybored 2y ago
  This is so puzzling to me. It’s like people[1] get so stuck in the GitHub PR framework/mindset that they then, after realizing that GitHub PRs suck for reviewing individual commits, discard commits as the reviewable unit and then shoehorn one-commit-per-PR as a replacement. The PR is just the best GitHub had to offer. There are other approaches to code review. [1] Here we are generalizing.
  - skydhash 2y ago
    I treat PRs like the email workflow. You send a diff my way for a particular changes, I either accept it or reject it. Or I suggest modifications. Recursively. It’s the whole patch I’m interested in, not the implementation history (I’m not your tutor). Once approved, I make a commit in main for this diff.
    - keybored 2y ago
      The classic email workflow is either one patch or a patch series. Where each patch becomes a commit. And each patch can be reviewed in isolation (like a commit). It is not anemic like the squash workflow.
      - skydhash 2y ago
        There’s nothing stopping anyone from creating a PR series. My reasoning for the squash workflow is described here[0]. I just equate a PR to a patch. And it becomes a commit in the main branch. I don’t really care about the commits in the PR, just like no one cares about the commits that produced the patch in the email workflow. [0]: https://news.ycombinator.com/item?id=41839282 https://news.ycombinator.com/item?id=41839282

keybored 2y ago

> Is there anyone else that enforces a simple "just squash & merge everything from PRs into main" across the entire team? No. This kind of policy has plenty of downsides[1] and the upsides are relatively minor. [1] https://news.ycombinator.com/item?id=41653892 https://news.ycombinator.com/item?id=41653892

skydhash 2y ago
The upsides are great. PRs should be atomic, introducing changes as a set. I don’t care about the history of the branch, I only care that the changes in the PR are one unit that can be reverted as a whole. And each commit in the main branch passes the tests.
- keybored 2y ago
  I get those so-called upsides with `git log --first-parent`. I can view the changes as one set of changes or all of the individual ones. But I can’t relate in general to those workflows where PRs have to be tiny and revertable (why need to revert so often).
  - skydhash 2y ago
    It’s mostly about project management. In a project, the unit is a task. These tasks will then have discussion around then and then get assigned to the people that are going to do the implementation. After the latter is done and approved, it will be deployed and the task will be marked as done. Having your git history reflecting this is great when you need to take decisions. Because no one will care about your individual commits. Every discussion will revolve about the task. So you make the PRs a bijective relation with it, and by extension the commits on the main branch. Then for special group of tasks like features, you have a specific branch you manage the same way. The other branches you create PRs from can be as messy as you like. And your local copy is yours to do whatever you want.
    - keybored 2y ago
      I know what project management and tasks are. Commits are in practice a superset of all of that. Some commits will correspond to a task. But many commits will just do the work without having to go through the beaurocracy of “project management”. Having to “project manage” each and every commit if you want a useful and fine-grained (but not micromanaged) version control history is impractical. There’s a lot of small tasks in the concrete code. It doesn’t make sense to give every little change a task. And the flipside of that is that you can give up on making commits that are fine-grained enough. Now you’re back in squash town where, sure, your task list is just right. But your commits are sometimes a jumble of different concerns in order to stick to one-task-on-commit. > Because no one will care about your individual commits. Will someone care about your tasks? You don’t pre-empt what commits you care about when you are trying to find the reason for some change in the Git history. You don’t find the exact commit that explains it, clear as day, and then throw it away because there is no associated task. > So you make the PRs a bijective relation with it, and by extension the commits on the main branch. It sounds useful with a mathematical name I guess? Insisting on one-to-one mappings for such small things like commits smells of micromanagement. Yes, either people-management or your own time management.
      - skydhash 2y ago
        > Having to “project manage” each and every commit if you want a useful and fine-grained (but not micromanaged) version control history is impractical. You're misunderstand me there. I'm talking about the main branch's commits, on the main repository, not every branch and every commits, and not the local repos. And it's not dogmatic, just very nice to have. If you want a hotfix, it's very easy to create a new branch, commit the fix, push it, create a PR, and squash merge it. Unless you like to edit "main" directly on a collaborative project. Or you can then instruct everyone how to cleanup their commits and make sure that each one is atomic. And you can merge your usual way. I have no dog in this fight. It's just that the above is easier to do. It's pretty much the same result.
        keybored 2y ago
        > You're misunderstand me there. I'm talking about the main branch's commits, on the main repository, not every branch and every commits, and not the local repos. No I don’t. I’m also talking about the commits that go into the main branch. The “permanent history”, not all the commits that happen on feature branches. > And it's not dogmatic, just very nice to have. A squash-only policy is by definition dogmatic. That’s the only thing that I’ve argued against here—a policy. Having the freedom to do it or not is totally different. > Or you can then instruct everyone how to cleanup their commits and make sure that each one is atomic. And you can merge your usual way. I have no dog in this fight. It's just that the above is easier to do. It's pretty much the same result. No. OR you can let people do what they want. Let them squash if they want. If squashing is such a win they can do that. No instructions necessary.

Chromozon 2y ago

My job follows this, and "rebase" isn't even in the vocabulary at work. It's just so simple- our developers spend zero time with git issues because there aren't any. The master branch history ends up being a sequence of pull requests that all have a corresponding code review. Everything stays nice and tidy.

keybored 2y ago
Rebase isn’t in our vocabulary either. Because those of us who use it just do it. And those that don’t, don’t. And so there is no reason to interfere with anyone’s way of working.

Degorath 2y ago

But that way you make stacked diffs/changes/PRs really hard, so is it really worth it? I firmly believe GitHub makes reviewing individual commits in a PR so painful that I'd rather not do that.

dbalatero 2y ago

Yep, IMO this is always the best of the 3 strategies. The PR is the unit of work, people get too hung up on the PR's individual commits. Worst: rebase and merge - you end up with your coworker's broken WIP commits all over master, and have a terrible git revert story OK: merge commit - you can revert, but there is a less intuitive `-m 1` flag (IIRC?) you have to pass into revert, and IMO you rarely need the intermediate history. Best: squash & merge - you get one single commit representing the unit of work merging in, git revert is dead easy Also setting the commit message in main to the `{title} (#{prNum})\n\n{prDescription}` format preserves all the good context from your PR and lets you get back to it if you need.

keybored 2y ago
Three options? The best alternative to squash-merge is to freely rebase before you merge. Or for that matter squash if you just end up with one commit (because rebase subsumes squash). 1. You get rid of WIP commits, typo fix commits, all kinds of transient changes (for review and so on) 2. You keep the substantive ones 3. All changes are logically separated I don’t see where you cover this option. > Also setting the commit message in main to the `{title} (#{prNum})\n\n{prDescription}` format preserves all the good context from your PR and lets you get back to it if you need. Again. It is ironic that people are so comfortable with dumping the history in a webapp when you are working with a version control system. I am fine with a lot of the history being on GitHub or whatever other webapp. The review history, that is. But I’m not comfortable with leaving the history of the pre-squashed commits on GitHub if those pre-squashed commits were useful for the long-term history.
- kccqzy 2y ago
  If the code author deems the pre-squashed commits useful, they would've split their PR into multiple PRs. I personally do this all the time: each PR has only one commit, but PRs can depend on each other. And if the PR author deems their commits insignificant, they can feel free to make one PR and they squash & merge.
  - keybored 2y ago

kccqzy 2y ago

Yes after I recently discovered that even with squash & merge, the intermediate history isn't lost: they are not in the git history but they can still be viewed on GitHub.

mkesper 2y ago
But then git isn't your source of truth anymore. GitHub still references these commits until they're garbage collected one day.
- kccqzy 2y ago
  My understanding is that these commits don't get garbage collected ever. After all they still need to support `gh pr checkout` so they need to store them forever as part of the GitHub repo. Sure they won't be included in `git clone` but that's kind of the point.
  - keybored 2y ago
    That’s correct. They will not ge GCd. And yet that’s not usually what the problem is with having your canonical history “over there” in a proprietary webapp. But if the history is easily available on those PR refs then I guess it is (under doubt) fine.
    - skydhash 2y ago
      If you really care, you can use the API to extract the PR’s data in a local archive.
      - keybored 2y ago
        If I really care about the version control history? Yeah. Alternatively I could stick with git(1) which does all of this stuff out of the box. Instead of having to learn a superflouous (for the job) API.

zabil 2y ago

We do this. We deploy on commit to main and use the commit SHA for versioning. So squash merges makes it easier to know which commit caused a bug or bisect. This work well for our PR based workflows. If required the PR can be used as reference for un-squashed commits.

keybored 2y ago
If you deploy on merge (squash or regular) to main then having the commits squashed makes no difference with regards to finding out what was deployed.

deleted 2y ago

[deleted]

AnthOlei 2y ago

I’ve recently spent time truly learning git, and I’m realizing how much better it is to take care of your commits in a PR even if you squash to main. My reviewers love when I write “review commit by commit” in the PR description. Then each individual commit has its own reasoning, and you can mentally switch into reviewing if that commit does it’s one thing it’s supposed to do correctly. I will accept the argument that each commit should be its own PR though :)

keybored 2y ago
This is so puzzling to me. It’s like people[1] get so stuck in the GitHub PR framework/mindset that they then, after realizing that GitHub PRs suck for reviewing individual commits, discard commits as the reviewable unit and then shoehorn one-commit-per-PR as a replacement. The PR is just the best GitHub had to offer. There are other approaches to code review. [1] Here we are generalizing.
- skydhash 2y ago
  I treat PRs like the email workflow. You send a diff my way for a particular changes, I either accept it or reject it. Or I suggest modifications. Recursively. It’s the whole patch I’m interested in, not the implementation history (I’m not your tutor). Once approved, I make a commit in main for this diff.
  - keybored 2y ago
    The classic email workflow is either one patch or a patch series. Where each patch becomes a commit. And each patch can be reviewed in isolation (like a commit). It is not anemic like the squash workflow.
    - skydhash 2y ago
      There’s nothing stopping anyone from creating a PR series. My reasoning for the squash workflow is described here[0]. I just equate a PR to a patch. And it becomes a commit in the main branch. I don’t really care about the commits in the PR, just like no one cares about the commits that produced the patch in the email workflow. [0]: https://news.ycombinator.com/item?id=41839282 https://news.ycombinator.com/item?id=41839282