67

If I come across a non-critical typo in code (say, an errant apostrophe in a print(error) statement), is it worth making a commit to resolve that error, or should it simply be left alone?

Specifically, I'm curious about weighing the gumming-up of the commit log against the value of resolving these non-critical typos. I'm leaning toward resolving them. Am I being pedantic?

2
  • 40
    If a trivial commit gums up anything, you either need to invest in a better VCS, better log-filtering tools, or better practices for notating different severities of fix.
    – Rex Kerr
    Jul 10, 2012 at 18:11
  • @root45 Although if you look at the results for it, they're all a different type of grammar than this question is asking about.
    – Izkata
    Jul 10, 2012 at 18:40

10 Answers 10

133

My personal feeling is that improving quality is worth the minor inconvenience of an additional commit log entry, even for small improvements. After all, small improvements count a lot when you factor in the broken window effect.

You might want to prefix it with a TRIVIAL: tag, or mark it as trivial if your VCS supports it.

11
  • 73
    Just want to add to this is that I'd much rather stuff like this be committed separately than lumped in with something else. The problem with lumping is that too much noise can distract a code reviewer from the relevent changes. +1
    – Andy
    Jul 10, 2012 at 16:03
  • 9
    +1 for mentioning the broken window effect. Small things matter. If everything is really clean, people will think twice before committing carelessly written or untested code.
    – Roy Tinker
    Jul 10, 2012 at 16:47
  • 26
    Also if you lump it in with a feature, then role back the feature you loose the change. Commit one thing at a time. Jul 10, 2012 at 21:02
  • 4
    I'd be really interested which VCSs allow for marking comment as trivial. I've worked with SVN, Hg and Git, and haven't noticed anything like that in either.
    – Xion
    Jul 11, 2012 at 7:34
  • 3
    @Andy that noise isn't the worst part... if someone later decides to for example revert this commit because they don't want that feature, suddenly you also lose a small improvement that was part of the commit(s).
    – rFactor
    Jul 11, 2012 at 12:39
49

You aren't being pedantic, and it is better to resolve them individually. The more atomic a change is, the better -- you don't want a crashing bug fix to be mixed up with 500 comment/typo changes.

1
  • 9
    +1 Each revision should relate only to one specific fix. It becomes a NIGHTMARE to backport fixes if each revision also has a bunch of random typos fixed in between real changes.
    – Grant
    Jul 10, 2012 at 15:41
29

In the General Case: Yes

It's always worth it to increase the maintainability of your software.

Just go for it.

If you're just about to ship a release...

... and if you're not the team leader, then check with him/her.


Regarding the content of the commit log...

I agree with others that you should at least write something make it distinct from "feature"-related commits, if it's only about fixing an isolated typo.

A common practice is to have some never-dying tasks in your issue tracker to track timeless and endless changes. For instance, it's not uncommon to have a task for:

  • large automated harmless cleanups (whitespaces sweeps),
  • grammar and typo hunting sprees,
  • build system modifications,
  • etc...

Just be cautious that these do not get used as throw-away task IDs for just about anything when people get lazy about creating correctly documented tickets. Especially if you reject commits that are not linked to an ID (which is a good thing, but large tasks like these will be even more attractive for lazy developers).

3
  • 7
    Checking with your team leader for a typo fix? If I was the team leader who's stressed up with the upcoming release, I wouldn't want to be disturbed with such trivialities.
    – Joh
    Jul 11, 2012 at 9:13
  • 2
    @Joh: it's not that you might necessarily break the build, but there might be other build jobs, or one might be already tagged and ready for use, and your audit controls (depending on your industry) might force you to tie this harmless commit to tasks and requirements, so if it comes out of nowhere that can just be a bit of a headache; which you could easily save for just a few days after the release is out. But in general, I'd agree with you, and I'd even say that a company running like that is doing it wrong. But you're not the one who'll fix that, so run it by them if you're not senior.
    – haylem
    Jul 11, 2012 at 11:30
  • 2
    @Joh: basically, if my team lead sees a new build suddenly jumping up on the build system when he's about to start a new build (or already started it), then I can assure you his stress levels will be up by using this approach as well...
    – haylem
    Jul 11, 2012 at 11:32
7

Typos should be added as a commit. Fixing misspelled words or grammatical errors will increase the readability of your code.

Using a commit message such as "Fixed typo" or "Fixed typo in file.c" will help you distinguish these commits from other, major code commits.

7

Yes, you should absolutely do this, particularly early on in a project.

Why? Two points:

  1. You likely won't know if a typo is "critical" or not until it is too late. Some fixes probably don't get fixed because everyone thinks it won't be a big deal. Until it is.

  2. Fixing a typo early on and deliberately will be much easier than fixing it after several hundred lines of code/function calls have been made with it. Again, temporary hacks can become semi-permanent surprisingly quickly. This is why I have to deal with objects that have both "CollapseAll" AND "ColapseAll" methods.

1
  • 2
    Yes - of all the correct answers, I like this one best for mentioning that it depends on the "when." Jul 10, 2012 at 19:00
4

For grammatical errors that may be seen by an end user, then yes, by all means it is worth making the commit as it is entirely possible that a user or QA may come along and report the error and it would need to be tracked. If it is already fixed then it could expedite the time it takes to resolve the issue.

If it is a grammatical error in comments around the code though, I wouldn't do anything about it unless it is part of changes to the actual code as well in which case you are updating the documentation of the code.

3

If you're worried about gumming up the commit log, then you're doing something else wrong. :-) Frequent commits are a good thing! I commit typo fixes all the time. Get 'em in the codebase ASAP and speed up the dev cycle!

2
  • 2
    I commit typo fixes all the time IMO that's worrisome, try to find programmers with better English?
    – Lie Ryan
    Jul 11, 2012 at 4:36
  • Ya take what ya can get these days. Finding programmers that can write workable code at all is a serious problem! Jul 11, 2012 at 15:07
2

I vote yes. Check them in. I've worked for a company that hated people checking stuff in. I mean almost anything.The code reviews were extensive and so if you checked in a change that just fixed a typo you got moaned at. You can imagine the state of the code. Actually the code wasn't dreadful, but it oozed like treacle rather than flowed like wine.

0

Does your change management process allow it?

In my environment, every commit I make must be tied back to a change request which has been requested either by the business users or a mandatory system-level change, complete with the corresponding end-user testing processes. A simple typo like you're describing probably wouldn't be logged as either of those (I've found typos & grammatical errors in one of my apps which existed for over 4 years without anyone noticing), so if/when the auditors came calling I'd have a very difficult time explaining myself.

I would save a change like you're describing (and actually have one, as a matter of fact - a method name is mis-spelled and I just discovered it) for a time when I have a "real" change that needs to be made as well and put "fixed various typos" in the log.

6
  • +1 In some environments this is true. Please don't downvote just because this isn't true where you work.
    – MarkJ
    Jul 12, 2012 at 19:34
  • -1 your change management process seems to suck big time. I would understand (and even prefer) that every commit has to be related to change request - this part of your process looks fine. But OMG it looks like developer-initiated requests (like refactor this / fix typos in that) are prohibited which raises a big red flag to me. It kind of assumes business users / auditors always know better than developers - if that would be ever close to truth, then they'd write the code themselves
    – gnat
    Jul 17, 2012 at 15:17
  • If the developer can convince the people who approve going forward with the changes that they're worthwhile to make, then they can move forward. But if I find a simple typo in the name of a method, or find some code that's copy/pasted which should be refactored into a method, I cannot make the change without authorization. It's not simply about "doing the right thing with the code" - acceptance testing has to be performed, and developers aren't in a position where they can say to the business people "I made this change, you'll never notice what I did, but you have to go through a test cycle."
    – alroc
    Jul 17, 2012 at 16:56
  • @alroc that "if-can-convince" just hides a counter-productive process under a reasonably looking surface. Requirement to convince business / audit about big changes, about agreed milestone checkpoints / releases etc etc makes good sense, OK. Spending a few hours / days to justify an effort that takes a week of dev and QA, that's tedious but fair, OK. But forcing this for every single spelling fix or extract method... give me a break - this is nothing but control mindlessly inserted at the wrong stage in the wrong time
    – gnat
    Jul 17, 2012 at 23:31
  • Let me try explaining this again. If I can correct a typo or extract a method while working on another, approved change, I can slip it in without having to sell anyone on it. But I cannot make a change that isn't user-visible and doesn't have a direct positive impact on the business/users without selling the powers that be on it first.
    – alroc
    Jul 18, 2012 at 10:55
-1

Use DVCS to edit history

If you are concerned about a clean commit history, consider doing your main work in feature branches. If you happen to work with a distributed VCS, you can easily edit your commit history before pushing it to the main branch. If you are on SVN, try Git -- it can interact bidirectionally with Subversion, and you can also edit history before actually committing to Subversion.

Keep common sense otherwise

If you don't want to or cannot edit commit history, there is no functional reason to do an early or an atomic commit for a minor typo that does not affect automatic tests or compilation. In this cases, in my opinion, keeping the commit history clean should be more important than doing really atomic commits. Mixing one or two typo corrections with a "regular" modification won't harm any potential review process. You might however want to group several trivial corrections into one commit, perhaps when "cleaning up" after a bigger coding session.

Note that functional bugs still should be committed ASAP in an atomic commit.

The general tone of the answers here seems to suggest a "commit everything fast" strategy even for minor typos. I tend to disagree and welcome discussion.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.