back

by jasonpeacock·5y ago·view on hn ↗
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.

As for getting distracted and refactoring something else - you're context switching. When you do that, you can also context-switch your repo by stashing your current changes (git stash) while you work on that tangent. Or commit your current changes to their local branch (you're liberally using branches, right? Branches are cheap in Git) and switch to a new branch for refactoring.

If you really want to write your commit first (which is a great idea, because you should know what changes you're planning to make before you start working on them!), then you can create an empty commit when you switch to your new feature branch:

    git commit --allow-empty
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.

Most issues I see from people getting confused with their changes and using Git is not using enough branches for their work, not being practiced at branch manipulation, and not being disciplined and intentional about their changes. People aren't thinking about "how am I going to release this?" or "how can this be reviewed most efficiently?", and they get carried away with making a lot of changes at once instead of many small changes.

My rule of thumb is that I'm only allowed to take one step off the path/plan, anything else gets put on a todo list to revisit later.

Plan the work, then work the plan.

[1] https://www.informit.com/articles/article.aspx?p=1235624&seq...

8 comments
> 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.

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.

>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.

> They don't do zero harm though. The git blame history gets thrashed.

That's exactly it; the keyword here is 'churn'. There was one OS project (I'm sure there's more) that basically said that small (typo, style) pull requests will not be accepted for that very reason.

But that's where gratuitous code review, tooling, and rebasing come in; ideally a feature is complete and reviewed and tidy before it gets merged into master and 'solidified' as part of a project's history. Ideally anyway.

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.

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.

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.

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.

> you're liberally using branches, right? Branches are cheap in Git

Branches may be cheap in Git, but they're expensive in typical modern software eng. process. For every branch, a PR must be opened, code must be reviewed, full test suite must be ran, the testers must ok that the change does not require extending the test suite, and only finally after the whole ordeal we're ok to merge. That's why I prefer my branches big and meaty.

> For every branch, a PR must be opened, code must be reviewed, full test suite must be ran, the testers must ok that the change does not require extending the test suite, and only finally after the whole ordeal we're ok to merge.

Surely this isn't true for local branches on your machine? I hope you're not living under such a dystopia.

But this process is small and fast when your branches are small and fast.

Code reviews length takes exponentially longer and misses way more when branches are "big and meaty"

It should be small and fast, but depending on where you work it isn't. And trying to get a big corp to adapt to you is like trying to boil the ocean.
Just create the branch locally for your incremental changes and then when you are done, connect the remote repository to initiate the PR.
I didn't know about the empty commit, that's useful.

The approach I try to follow is below. The BLUF is I use a similar but more granular approach and agree with writing the commit before you do the work.

First, I write the commit message capturing the intended design in a file and then try to create a skeleton of API level changes with method names and stubs from my design as my first commit. I use the file as my commit message for this.

Next, as I work I commit freely locally with small commits but I don't push to remote. I do this rather than commit --amend so I can track and revert as needed if needed. I probably could just use --amend.

Last, once I have it working I squash those commits into one commit and push to remote, then merge in changes from master if any and repush, and then start cycle over.

I tend to do this at one level below the issue I am working, so I then still have multiple commits for an issue, one per sub-issue. I will sometimes leave these as is in the PR, or sometimes I will squash these into a single commit as well. Which I do depends on how active the branch I'm targeting with the PR is, how big the commits are, and the extent of the changes.

Really depends on other changes. Say you change some variable name since whoever was there last forgot to rename something while they did refactoring. Couple days later someone fixes a bug that is related to this variable in some other way. Now you can't simply back port the bug fix commit into earlier release because it relies on a commit that introduces a feature that is not wanted/needed. Obviously fix is simple enough either edit the commit to also contain the variable change or make a tiny "dummy" commit that just contains the fix.

Now whoever is making this decision is cursing you since you could have just made the dummy commit in the first place and prevented this extra work that went into figuring out when the bug fix commit alone wasn't working/compiling.

> As for getting distracted and refactoring something else - you're context switching.

Which absolutely should be a new branch and probably a different ticket.

In addition to empty commits or rebasing work in progress commits, git stash accepts -m/--message and you can write a "proper" commit message when stashing work in progress. Some client apps (I've seen VSCode do it) will even kindly move that message from the stash into their commit message writing box on popping the stash.
My only issue is that if you fix something small and include that in an unrelated feature or bug fix commit, is if you have to do a "git revert" you are then reverting both items.
That's why you don't squash before merging. If you merge the original commits (and make a separate commit for the minor fix), you can revert the problematic commit without affecting the rest of the feature.
But making a separate commit for the minor fix is onerous once your working copy has accumulated other changes. That's what the OP says -- you have to use "git add -p" and that is onerous.
Why would something with such a good concept need to be tied to a gendered term?

How about calling it "camp grounding"?

The Boy Scouts of America did a lot to promote Leave No Trace and helped it become a common practice.
It's not appealing to any kind of gendered stereotyping though. It's a quote from the founder of "The Boy Scouts Association".
I suggest you take your complaint to the English language office. In Dutch, that organization would be called "padvinders" (lit. pathfinders, i.e. trackers) which shows that the concept itself isn't gendered.
Because it's a Boy Scout rule?
Your suggested term works, too. But why is it bad that a good concept has a gendered term?
The non-gendered term for "Boy Scouts" is simply "Scouts" or "Scouting" [1]. So if you need a non-gendered name, just call it "Scouting".

And the word 'scouting' already means "moving ahead of a group to gather information", which makes sense in the software engineering context too.

[1] https://en.wikipedia.org/wiki/Scouting