5 ms·

I do agree that PRs should have a clean history. However: - Services like GitHub allow you to restore the original branch, so you never actually lose history.

by ginja 5y ago

I do agree that PRs should have a clean history. However: - Services like GitHub allow you to restore the original branch, so you never actually lose history. So I don't see any major drawbacks of squashing on merge. - If your PRs are several thousand lines long, they probably should've been broken up into multiple PRs (your reviewer will appreciate it)

finnh 5y ago
Spot-on for both counts. Preserving a PR's individual commits in the master branch is insane - many of those commits won't represent a fully working system anyway, so why keep them in master?
- sjoruk 5y ago
  > many of those commits won't represent a fully working system anyway This is entirely within the control of the developer(s) working on the project. Whether it's worth it is up to the people working on it. It's certainly not insane though - it's much easier to fix merge/rebase conflicts when each commit is small and easy to reason about.
  - finnh 5y ago
    > it's much easier to fix merge/rebase conflicts when each commit is small and easy to reason about. I agree, but I'd say it in the context of PRs. It's much easier to fix issues (and avoid them) when _PRs_ are small & easy to reason about.
- gouggoug 5y ago
  It's entirely not insane _on the premise that the history is clean_. That's the whole argument that is being made here. If you think that "Preserving a PR's individual commits in the master branch is insane - many of those commits won't represent a fully working system anyway", your issue _is_ "that your commits don't represent a fully working system anyway", _that_ is what you need to fix. You need to make sure your history _does_ represent multiple small digestible atomic chunk of codes, that are understandable individually. By squashing, you're not fixing the root cause, you're treating the symptoms.
pizza234 5y ago
> - Services like GitHub allow you to restore the original branch, so you never actually lose history. So I don't see any major drawbacks of squashing on merge. There's definitely a drawback, and it's bisecting, which is actually a big deal. Bisecting allows in a semi-automated (depending on the issue) bisecting what otherwise can be a large diff. But of course, it requires a disciplined history - otherwise, bisecting just won't work.
- marcinzm 5y ago
  Can't you just bisect twice. Once to find the PR and a second time within the PR's branch?
  - pizza234 5y ago
    AFAIK, if you squash merge, the SCM (intended as the repository, both local and remote) will keep only a single commit; the commits breakdown will be in GitHub's (intended as a service) history.
gouggoug 5y ago
So, if your PR has a clean history in the first place (clean as in multiple small digestible, atomic commits, that are easy to review in isolation), why get rid of it with a squash? Is there a fundamental drawback to keeping that clean history? _It's already here_

by ginja 5y ago

finnh 5y ago

Spot-on for both counts. Preserving a PR's individual commits in the master branch is insane - many of those commits won't represent a fully working system anyway, so why keep them in master?

sjoruk 5y ago
> many of those commits won't represent a fully working system anyway This is entirely within the control of the developer(s) working on the project. Whether it's worth it is up to the people working on it. It's certainly not insane though - it's much easier to fix merge/rebase conflicts when each commit is small and easy to reason about.
- finnh 5y ago
  > it's much easier to fix merge/rebase conflicts when each commit is small and easy to reason about. I agree, but I'd say it in the context of PRs. It's much easier to fix issues (and avoid them) when _PRs_ are small & easy to reason about.
gouggoug 5y ago
It's entirely not insane _on the premise that the history is clean_. That's the whole argument that is being made here. If you think that "Preserving a PR's individual commits in the master branch is insane - many of those commits won't represent a fully working system anyway", your issue _is_ "that your commits don't represent a fully working system anyway", _that_ is what you need to fix. You need to make sure your history _does_ represent multiple small digestible atomic chunk of codes, that are understandable individually. By squashing, you're not fixing the root cause, you're treating the symptoms.

pizza234 5y ago

> - Services like GitHub allow you to restore the original branch, so you never actually lose history. So I don't see any major drawbacks of squashing on merge. There's definitely a drawback, and it's bisecting, which is actually a big deal. Bisecting allows in a semi-automated (depending on the issue) bisecting what otherwise can be a large diff. But of course, it requires a disciplined history - otherwise, bisecting just won't work.

marcinzm 5y ago
Can't you just bisect twice. Once to find the PR and a second time within the PR's branch?
- pizza234 5y ago
  AFAIK, if you squash merge, the SCM (intended as the repository, both local and remote) will keep only a single commit; the commits breakdown will be in GitHub's (intended as a service) history.

gouggoug 5y ago

So, if your PR has a clean history in the first place (clean as in multiple small digestible, atomic commits, that are easy to review in isolation), why get rid of it with a squash? Is there a fundamental drawback to keeping that clean history? _It's already here_