4 ms·

> Fixing a typo as you work on something else is not bad - that's called Boy Scouting[1] and it's totally acceptable to sprinkle small improvements/cleanups in

by bilalq 6y ago

> Fixing a typo as you work on something else is not bad - that's called Boy Scouting[1] and it's totally acceptable to sprinkle small improvements/cleanups in a change. Different people have different standards and opinions on what's acceptable. Personally, I'm in the camp of people that would hate to see an unrelated typo fix being included in the same commit as some other change. I strive for semantically meaningful commits, and prefer the people I work with to do the same. It's fine to make such fixes, but they should be in separate commits (though they can be in the same code review request). The pain point the author of the article mentioned is very real (needing to do `git add --patch`). > Write your "plan" as the first commit in that branch. Then do your work, committing frequently, and when you're done and squashing your changes into a single commit for review you now have that first commit available and waiting. There are often times when multiple end commits make sense. Squashing/rebasing down to essentials is fine, but that still may be more than just one commit on occasion.

watwut 6y ago
Most people however dont mind them at all and they practically do zero harm. However, forcing people to split it into two commits likely means it wont ever get done. Because who wants to go through previous commit files to check for typos in other peoples code. People wont, either they fix it as they are noticing them or not at all.
- bilalq 6y ago
  >Most people however dont mind them at all and they practically do zero harm. They don't do zero harm though. The git blame history gets thrashed. If I see the last change one line 15 of a file is associated with a commit titled `Enable TCP connection reuse`, I'm going to wonder what a change to a line throwing an unrelated exception has to do with that. This means I'll waste time investigating a red herring. If I need to see the blame to find out who to ask about the feature, I also get garbage direction. The same preference for semantic commits applies to things like optimizations. Don't sneak in keep alive timeout changes in the same commit as `Add email notification support`. If a bug turns up on the email feature, I don't want to waste time potentially looking at the wrong thing. > However, forcing people to split it into two commits likely means it wont ever get done. That's not been my experience. They either get corrected at code review time on first release, or someone will make a semantically isolated commit for the fixes and include them in a future code review/pull request. > Because who wants to go through previous commit files to check for typos in other peoples code. People wont, either they fix it as they are noticing them or not at all. Not sure what point you're making here. You don't need to go through previous commits to see typos if they're still there in the current commit.
  - watwut 6y ago
    You have to go through them to find the typos again. So you wont. The harm to history is marginal. It is quite unlikely that you fixed that many typos in all surrounding lines. I have yet to be confused by history like that.
    - bilalq 6y ago
      You can fix them as you find them. Just save as a separate commit. No need to go back to find anything. Typos were just an example. There are other changes that are just as harmful to history that people might otherwise think are safe to squeeze in. Examples include whitespace changes and minor refactors. All of these can really mess with history and make things confusing. Refactors in particular really mess with your head and can even make the code review step confusing if the review wasn't sent with multiple commits. Keeping semantic isolation of commits has value in its neatness. You may be able to get by without doing so, but you'll be better off if you did.
      - watwut 6y ago
        So I will commit half file in one comit and second in another? Really not worth the risk not time. History is not that epically useful that we should avoid cleanups.
        bilalq 6y ago
        Yes, you can commit parts of a file at a time. This is easily done by using `git add -p`. This is a subjective matter, so I don't think your point of view here is wrong or anything, but I do disagree with it. History is something I've consulted many, many times on large code bases. It's helped save precious minutes while triaging situations where an alarm went off and customer was ongoing.
        bilalq 6y ago
        customer impact*

by bilalq 6y ago

watwut 6y ago

Most people however dont mind them at all and they practically do zero harm. However, forcing people to split it into two commits likely means it wont ever get done. Because who wants to go through previous commit files to check for typos in other peoples code. People wont, either they fix it as they are noticing them or not at all.

bilalq 6y ago
>Most people however dont mind them at all and they practically do zero harm. They don't do zero harm though. The git blame history gets thrashed. If I see the last change one line 15 of a file is associated with a commit titled `Enable TCP connection reuse`, I'm going to wonder what a change to a line throwing an unrelated exception has to do with that. This means I'll waste time investigating a red herring. If I need to see the blame to find out who to ask about the feature, I also get garbage direction. The same preference for semantic commits applies to things like optimizations. Don't sneak in keep alive timeout changes in the same commit as `Add email notification support`. If a bug turns up on the email feature, I don't want to waste time potentially looking at the wrong thing. > However, forcing people to split it into two commits likely means it wont ever get done. That's not been my experience. They either get corrected at code review time on first release, or someone will make a semantically isolated commit for the fixes and include them in a future code review/pull request. > Because who wants to go through previous commit files to check for typos in other peoples code. People wont, either they fix it as they are noticing them or not at all. Not sure what point you're making here. You don't need to go through previous commits to see typos if they're still there in the current commit.
- watwut 6y ago
  You have to go through them to find the typos again. So you wont. The harm to history is marginal. It is quite unlikely that you fixed that many typos in all surrounding lines. I have yet to be confused by history like that.
  - bilalq 6y ago
    You can fix them as you find them. Just save as a separate commit. No need to go back to find anything. Typos were just an example. There are other changes that are just as harmful to history that people might otherwise think are safe to squeeze in. Examples include whitespace changes and minor refactors. All of these can really mess with history and make things confusing. Refactors in particular really mess with your head and can even make the code review step confusing if the review wasn't sent with multiple commits. Keeping semantic isolation of commits has value in its neatness. You may be able to get by without doing so, but you'll be better off if you did.
    - watwut 6y ago
      So I will commit half file in one comit and second in another? Really not worth the risk not time. History is not that epically useful that we should avoid cleanups.
      - bilalq 6y ago
        Yes, you can commit parts of a file at a time. This is easily done by using `git add -p`. This is a subjective matter, so I don't think your point of view here is wrong or anything, but I do disagree with it. History is something I've consulted many, many times on large code bases. It's helped save precious minutes while triaging situations where an alarm went off and customer was ongoing.
        bilalq 6y ago
        customer impact*