4 ms·

Couldn't disagree more. Having worked extensively on teams on both sides of this issue, I can experientially state that a well-done git rebase and commit strate

by geuis 11y ago

Couldn't disagree more. Having worked extensively on teams on both sides of this issue, I can experientially state that a well-done git rebase and commit strategy is much more useful and helpful. In terms of feature branches: The individual engineer is free to do individual commits in their branch as they need to in order to keep track of their work. Before they submit a pull request, they should rebase and squash all of their commits into a single one that thoroughly describes everything in the feature that is being committed. When used in conjunction with tools like Phabricator, Arcanist, and commit templates, the workflow is very smooth. When another team member goes to code review their pull request, rather than having to examine multiple individual commits there is only a single one to examine and comment on. Master history: Rather than cluttering up the mainline history with 'Did this', 'Did that', 'Merged: Did this', 'Merged: Did that', 'Reverted: Merged: Did this' etc, you get a series of commits that articulately describe what each commit was for. In the event you need to revert a feature because it breaks something, its much easier to revert that single commit than trying to hunt through all of the individual commits from an engineers feature branch. And in that case, if you revert one of the commits from the feature branch it could break something else.

WorldMaker 11y ago
I'm only arguing that what you see as clutter, I see as potentially interesting history, and a mess of merges and reverts are more interesting than people give them credit. It can tell you quite a lot about a project. (Did you learn from reverting 'Merged: Did This' anything about why it shouldn't have been merged? Did you miss something in 'Merged: Did that' that didn't quite merge easily? That's easier to find/diagnose with smaller more often merges than big evil merges...) Anyway, to each their own, and I appreciate your preferences differ from mine.
- andrewprock 11y ago
  Here, that something might be "potentially interesting" is not worth the cost. In fact, it's probably a net drain on the organization. I once had a manager revert code formatting because he was "potentially interested" in having svn blame always show who wrote the actual code. In the meantime, it cost more time to develop because the code was impossible to read. When "potentially interesting" begins to impede productivity, the value of that policy needs to be established. Do you really want to incur development time cost (something that is always ongoing) just to make the code archaeology (something that does not need to happen very often) a little bit easier?
andersonvom 11y ago
I agree that a well-done git rebase and commit strategy is much more useful. However, squashing everything will (most likely) lead to gigantic commits that are hard to reason about. A better approach would be to create multiple small commits that work and are self contained. It's ok for commit N to depend on the preceding commit, but each N should be able to stand on its own. If devs rebase everything before pushing and push often (therefore also rebasing often), conflicts will happen a lot less often. Devs can also use their private branches for temporarily saving all WIP, squashing/rewording only what makes before submitting the PR or pushing to master.
teacup50 11y ago
Have you tried making sense of a project 10 years old? 20? 40? I can "experientially" state that squash throws away very necessary information for anyone trying to make sense of old code.
- cwilkes 11y ago
  that's more of how squash was applied in the situation ... If they had huge commits then while squash might break it up a bit that's a problem with how the people did their work. Myself I like to do PR and merges with code that can be logically comprehended quickly and doesnt break the build. a lot of that is due to being emphatic to people that are doing the reviewing. I know they don't know the context as they aren't in the code so I'm showing them a snippet of it, say a new method in s single commit. Then another commit for another method. Finally a third one that actually does some new functionality by combining the two. I figure that's good enough for people to see that unit tests passed on the two methods and that the system started getting errors after the last commit so to look there for faulty logic.
  - teacup50 11y ago
    It's rare that the contemporary author knows what will be useful context in two decades.

by geuis 11y ago

WorldMaker 11y ago

I'm only arguing that what you see as clutter, I see as potentially interesting history, and a mess of merges and reverts are more interesting than people give them credit. It can tell you quite a lot about a project. (Did you learn from reverting 'Merged: Did This' anything about why it shouldn't have been merged? Did you miss something in 'Merged: Did that' that didn't quite merge easily? That's easier to find/diagnose with smaller more often merges than big evil merges...) Anyway, to each their own, and I appreciate your preferences differ from mine.

andrewprock 11y ago
Here, that something might be "potentially interesting" is not worth the cost. In fact, it's probably a net drain on the organization. I once had a manager revert code formatting because he was "potentially interested" in having svn blame always show who wrote the actual code. In the meantime, it cost more time to develop because the code was impossible to read. When "potentially interesting" begins to impede productivity, the value of that policy needs to be established. Do you really want to incur development time cost (something that is always ongoing) just to make the code archaeology (something that does not need to happen very often) a little bit easier?

andersonvom 11y ago

I agree that a well-done git rebase and commit strategy is much more useful. However, squashing everything will (most likely) lead to gigantic commits that are hard to reason about. A better approach would be to create multiple small commits that work and are self contained. It's ok for commit N to depend on the preceding commit, but each N should be able to stand on its own. If devs rebase everything before pushing and push often (therefore also rebasing often), conflicts will happen a lot less often. Devs can also use their private branches for temporarily saving all WIP, squashing/rewording only what makes before submitting the PR or pushing to master.

teacup50 11y ago

Have you tried making sense of a project 10 years old? 20? 40? I can "experientially" state that squash throws away very necessary information for anyone trying to make sense of old code.

cwilkes 11y ago
that's more of how squash was applied in the situation ... If they had huge commits then while squash might break it up a bit that's a problem with how the people did their work. Myself I like to do PR and merges with code that can be logically comprehended quickly and doesnt break the build. a lot of that is due to being emphatic to people that are doing the reviewing. I know they don't know the context as they aren't in the code so I'm showing them a snippet of it, say a new method in s single commit. Then another commit for another method. Finally a third one that actually does some new functionality by combining the two. I figure that's good enough for people to see that unit tests passed on the two methods and that the system started getting errors after the last commit so to look there for faulty logic.
- teacup50 11y ago
  It's rare that the contemporary author knows what will be useful context in two decades.