8 ms·

Splitting up commits is totally underrated and seen rarely. I see colleagues over and over again plumbing 4 kinds of changes into the same commit. Good luck rev

Splitting up commits is totally underrated and seen rarely. I see colleagues over and over again plumbing 4 kinds of changes into the same commit. Good luck reverting the one change that caused an outage.

cedws 3y ago
Crafting good commits is a skill that nobody seems to care about. In fact, nobody really seems to care about learning git at all despite it being one of the main tools in software development.
- palata 3y ago
  I would even go as far as "crafting good software is a skill that nobody seems to care about". It feels like people care a lot about "being productive" though, hence tools like all the Copilots.
  - williamcotton 3y ago
    Those are mutually exclusive.
    - palata 3y ago
      Crafting good software and being productive? I guess it depends. I am much slower at producing code than most of my coworkers, but as a result they spend very little time maintaining/debugging it. But I spend most of my time debugging their crap (and they do, too). I think there is a point to make that better code is often more profitable in the longer term. But the norm (at least where I work) is to swim in crappy code all day and complain about it while producing more of it.
  - ollysb 3y ago
    The incentives are to get the code out the door then improve things later, if it's worth it. There's a lot of risk in product development, it iften doesn't make sense to craft the perfect software when there's a chance that it won't see the adoption that was hoped for.
    - palata 3y ago
      There is a difference between "crafting the perfect software" and "crafting good software". It is way too common to not realize it and settle on "crafting crappy software" instead. The problem being that crappy software is often very hard to improve later, if it's worth it. I have lived through multiple examples of this, the biggest one being a bad protocol (that was provably badly designed at the beginning) that got adoption, many tools built around it, and ten years later everybody hates it and keeps complaining but it's so hard to replace because of all the tooling built around it.
fuzzy2 3y ago
People will actually revert commits? For real? At work? Where do I get to sign up?
- spinningarrow 3y ago
  Relatively common practice where I work. What do you do instead?
  - shric 3y ago
    Not the GP but we tend to use decently small and meaningful commits to make PRs easier to review. They then get squashed, so you can only (easily) revert the whole PR.
    - williamdclt 3y ago
      Personally I’m happy with that. If the thing merged caused an issue, I’d want to revert the entire thing that was merged, not a subpart of it which would result in a state of the codebase that nobody reviewed. If you’re very diligent about making good atomic commits that always pass all test that can work, but I find that squashing PRs (and PRs being relatively small) is a very good trade off
      - matrss 3y ago
        Sounds reasonable. Also, when squashing a PR would conflate unrelated commits then maybe those commits should have been separate PRs instead. I've recently started to like squashing after usually preferring rebase merges, because with squashes I have an easy option to rewrite the commit message and edit out all of those meaningless "fix typo", "improve" and "now actually working" lines...
        palata 3y ago
        > I've recently started to like squashing after usually preferring rebase merges, because with squashes I have an easy option to rewrite the commit message And why couldn't you rewrite the commit messages while rebasing? EDIT: oh, you mean from the GitHub webUI, I presume. Got it.
chipweinberger 3y ago
just don’t spend more time splitting up commits than you would have reverting change usually you should err on the side of keeping the commits together, unless you’re close to release, patching a bug in release, etc.
jerpint 3y ago
Mostly it’s just that git makes it unnecessarily hard to do atomic commits across a file
quectophoton 3y ago
And on this topic, another underrated feature that everyone seems to hate is merge commits. If you use rebase on your own branch, but merge commits on the stable branch (whatever name you use for it), you get both things: * Small individual commits with the size and ordering that the original author intended on their branch. * A "commit group" that describes that set of changes for the sake of future maintainers' sanity, and that can be atomically reverted. More so in PR-based flows. You can have `git log --first-parent` display only the PR merge commits[1], and `git log --no-merges` display the linear history. [1]: Assuming you actually put useful info on those commits. If you use GitHub, those merge commits can be set to default to PR title and description.
- dayjaby 3y ago
  I also set up recently the policy to only use merge commits on stable branch, as otherwise the path filter^1 in the workflows would not detect correctly which files changed after merging a PR. [1] https://github.com/dorny/paths-filter https://github.com/dorny/paths-filter
yjftsjthsd-h 3y ago
> Good luck reverting the one change that caused an outage. Surely you'd revert to the last good release? I'm sure you could revert a single commit and re-release, but it seems slower (you have to find the right commit to revert, and make sure that the result works correctly) and riskier (since now you're effectively rolling forward to a combination of commits that's never been deployed before)

cedws 3y ago

Crafting good commits is a skill that nobody seems to care about. In fact, nobody really seems to care about learning git at all despite it being one of the main tools in software development.

palata 3y ago
I would even go as far as "crafting good software is a skill that nobody seems to care about". It feels like people care a lot about "being productive" though, hence tools like all the Copilots.
- williamcotton 3y ago
  Those are mutually exclusive.
  - palata 3y ago
    Crafting good software and being productive? I guess it depends. I am much slower at producing code than most of my coworkers, but as a result they spend very little time maintaining/debugging it. But I spend most of my time debugging their crap (and they do, too). I think there is a point to make that better code is often more profitable in the longer term. But the norm (at least where I work) is to swim in crappy code all day and complain about it while producing more of it.
- ollysb 3y ago
  The incentives are to get the code out the door then improve things later, if it's worth it. There's a lot of risk in product development, it iften doesn't make sense to craft the perfect software when there's a chance that it won't see the adoption that was hoped for.
  - palata 3y ago
    There is a difference between "crafting the perfect software" and "crafting good software". It is way too common to not realize it and settle on "crafting crappy software" instead. The problem being that crappy software is often very hard to improve later, if it's worth it. I have lived through multiple examples of this, the biggest one being a bad protocol (that was provably badly designed at the beginning) that got adoption, many tools built around it, and ten years later everybody hates it and keeps complaining but it's so hard to replace because of all the tooling built around it.

fuzzy2 3y ago

People will actually revert commits? For real? At work? Where do I get to sign up?

spinningarrow 3y ago
Relatively common practice where I work. What do you do instead?
- shric 3y ago
  Not the GP but we tend to use decently small and meaningful commits to make PRs easier to review. They then get squashed, so you can only (easily) revert the whole PR.
  - williamdclt 3y ago
    Personally I’m happy with that. If the thing merged caused an issue, I’d want to revert the entire thing that was merged, not a subpart of it which would result in a state of the codebase that nobody reviewed. If you’re very diligent about making good atomic commits that always pass all test that can work, but I find that squashing PRs (and PRs being relatively small) is a very good trade off
    - matrss 3y ago
      Sounds reasonable. Also, when squashing a PR would conflate unrelated commits then maybe those commits should have been separate PRs instead. I've recently started to like squashing after usually preferring rebase merges, because with squashes I have an easy option to rewrite the commit message and edit out all of those meaningless "fix typo", "improve" and "now actually working" lines...
      - palata 3y ago
        > I've recently started to like squashing after usually preferring rebase merges, because with squashes I have an easy option to rewrite the commit message And why couldn't you rewrite the commit messages while rebasing? EDIT: oh, you mean from the GitHub webUI, I presume. Got it.

chipweinberger 3y ago

just don’t spend more time splitting up commits than you would have reverting change usually you should err on the side of keeping the commits together, unless you’re close to release, patching a bug in release, etc.

jerpint 3y ago

Mostly it’s just that git makes it unnecessarily hard to do atomic commits across a file

quectophoton 3y ago

And on this topic, another underrated feature that everyone seems to hate is merge commits. If you use rebase on your own branch, but merge commits on the stable branch (whatever name you use for it), you get both things: * Small individual commits with the size and ordering that the original author intended on their branch. * A "commit group" that describes that set of changes for the sake of future maintainers' sanity, and that can be atomically reverted. More so in PR-based flows. You can have `git log --first-parent` display only the PR merge commits[1], and `git log --no-merges` display the linear history. [1]: Assuming you actually put useful info on those commits. If you use GitHub, those merge commits can be set to default to PR title and description.

dayjaby 3y ago
I also set up recently the policy to only use merge commits on stable branch, as otherwise the path filter^1 in the workflows would not detect correctly which files changed after merging a PR. [1] https://github.com/dorny/paths-filter https://github.com/dorny/paths-filter

yjftsjthsd-h 3y ago

> Good luck reverting the one change that caused an outage. Surely you'd revert to the last good release? I'm sure you could revert a single commit and re-release, but it seems slower (you have to find the right commit to revert, and make sure that the result works correctly) and riskier (since now you're effectively rolling forward to a combination of commits that's never been deployed before)