I was under the impression that a version control system eliminated the need to have "change logs" plastered everywhere in the code. I've often seen the continued use of change logs, including big long blocks at the start of stored procedures with a big section blocked out for changes to the file and littering the code with things like:

The reason for this, as it was explained to me, is that it takes too long to sift through our VCS logs trying to find who changed what and why, while having it in the code file itself, either at the top or near the relevant change, makes it easy to see who changed what and when. While I see the point of that, it seems redundant and just kind of smacks of "Eh we don't really understand how to use our VCS properly, so we won't bother with that stuff at all."

What do you think? Do you use both comments and the log? Just the log? Do you find that it's easier to code when you can see above a block of code that John Smith changed the method to check for XYZ a week ago, instead of having to search through logs and compare code files in a Diff tool?

EDIT: Using SVN, but basically only as a repository. No branches, no merges, nothing except log + storage.

Which version control system are you using?
–
ChrisF♦Jun 14 '11 at 14:10

11

Some shops used change logs before there were source control systems. Inertia and familiarity keeps the change logs around.
–
Gilbert Le BlancJun 14 '11 at 15:25

Ug, worse still, I've seen people put dated comments on nearly every line they change in a file, not just in the header. :(
–
Rodney GitzelJun 14 '11 at 21:55

2

+1 - My team also does this, and I tried to convince some of thems that it is the role of the VCS. The problems starts when these comments are not kept up-to-date with actual code... And the worst is that management decided to add changelogs to every method too.
–
kbokJun 14 '11 at 23:12

2

+1 - I've been fighting with our senior dev for almost two years about this. His only rationale for adding these useless comments is that it makes merging changes to 3000-line methods a little easier. The idea that a 3000-line method is obscene is met with scorn.
–
Joshua SmithJul 6 '11 at 14:16

12 Answers
12

I tend to delete comments in code. And by delete, I mean, with prejudice. Unless a comment explains why a particular function does something, it goes away. Bye bye. Do not pass go.

So it shouldn't surprise you that I would also delete those changelogs, for the very same reason.

The problem with commented out code and comments that read like books is that you don't really know how relevant it is and it gives you a false sense of understanding as to what the code actually does.

It sounds like your team doesn't have good tooling around your Version control system. Since you said you're using Subversion, I'd like to point out that there's a lot of tooling that will help you manage your subversion repository. From the ability to navigate your source through the web, to linking your changesets to specific bugs, you can do a lot that mitigates the need for these 'changelogs'.

I've had plenty of people comment and say that perhaps I'm in error for deleting comments. The vast majority of code I've seen that's been commented has been bad code, and the comments have only obsfucated the problem. In fact, if I ever comment code, you can be assured that I'm asking for forgiveness from the maintanence programmer because I'm relatively certain they'll want to kill me.

But lest you think I say that comments should be deleted in jest, this Daily WTF submission (from a codebase I worked on) illustrates my point perfectly:

/// The GaidenCommand is a specialized Command for use by the
/// CommandManager.
///
/// Note, the word "gaiden" is Japanese and means "side story",
/// see "http://en.wikipedia.org/wiki/Gaiden".
/// Why did I call this "GaidenCommand"? Because it's very similar to
/// a regular Command, but it serves the CommandManager in a different
/// way, and it is not the same as a regular "Command". Also
/// "CommandManagerCommand" is far too long to write. I also toyed with
/// calling this the "AlephCommand", Aleph being a silent Hebrew
/// letter, but Gaiden sounded better.

Oh... The stories I could tell you about that codebase, and I would, except that it's still in use by one of the largest government organizations around.

Whenever I struggle with a particular piece of complex logic, comments can sometimes help me get some context into why the logic is as convoluted as it is. But on the whole I agree with you.
–
maple_shaft♦Jun 14 '11 at 14:25

5

Every developer has at least a few bad qualities, I know I shouldn't but I just can't stop :) Everytime I create a TODO comment a puppy dies.
–
maple_shaft♦Jun 14 '11 at 14:39

30

Deleting other people's comments with prejudice is in a way forcing your own programming style and belief on others. Junior and pacifistic programmers will not bark back, but once in a while you run into an alpha male, and then a fight that should not have been begins. So, you have read on a smart blog of a smart person that when it comes to comments, less is more. Others might not have read the same book. Even if you think you are right because a few people on SO with 5k+ rep do concur, dictatorship is not the way to go about collaborative work. I worked under an alpha male before and quit.
–
JobJun 14 '11 at 16:25

@Neil Butterworth: re: ructions: Code is meant to be malleable. Refactor it, change it, improve it. It's meant for that. It's not meant to be written just once. Our understanding of the business changes and when that happens we need to change the code. I have a lot of problems with comments, but most relevant to our discussion is that comments rarely stay in sync with the code, and generally they don't explain why something is happening. When that happens, it's easy to delete it. If someone comes to me and asks why I deleted the following comment file.Open() // Open the file. I'd laugh.
–
George StockerJun 14 '11 at 17:13

Those "change-logs" embedded in the code are particularly naff. They just show up as yet another difference when you diff revisions, and one that you don't really care about. Trust your VCS - most have a "blame" feature which will show you very quickly who changed what.

Of course the really horrible thing was that feature of "old-time" VCS's where you could have the actual VCS log embedded in the source files. Made merging almost impossible.

Not only do they show up as yet another difference, worse yet they can also become wrong over time, whereas any good VCS will always be able to tell you who really wrote a particular line, as well as the history around that line. The only thing worse than useless comments are harmful comments. These comments start out as useless and eventually become harmful, if not totally ignored.
–
Ben HockingJun 14 '11 at 17:00

I have a single ChangeLog file for each project that is automatically populated by certain commit messages.

We don't have embedded ChangeLog comments. If we did I would remove them and have a "talk" with the person adding them. I think they are indicative of not understanding the tools you use, specifically the vcs.

We have a format for commit messages that makes it easier to grep the the logs. We also disallow useless or ambiguous commit messages.

The last company I worked at had software that had 17 years of history, development and annual updates behind it. Not all migrations from one version control system to the next would preserve comments or check-in notes. Nor did all developers over those years maintain any consistency with check-in comments/notes.

With comments in the source code, the archeological history of the changes were kept as notes, not commented out code. And, yes, they're still shipping VB6 code.

I personally abhor change logs within the code source file. To me is just feels like it violates a principle of software engineering in that any change I make has to be made in multiple places. A lot of times the change log information is completely useless and unimportant. In my humble opinion changes to the software should be documented when you check the code in.

But what do I know...

I think that if there is an insistance upon implementing the practice of keeping a change log within the source code that the change log should be limited to changes that affect the pulic API / interface of the class. If you are making changes within the class that are not going to break any code that uses it, recording these changes in a change log is just clutter in my opinion. However, I can see how it can sometimes be handy to be able to check the top of the source code file for documentation on any changes that may have broken anything for anyone else. Just a summary of how the change might impact the API and why the change was made.

On the other hand, my shop primarily does C# stuff, and we always use inline XML comments to document our API, so reading documentation on public API changes is more or less as simple as utilizing intellisense.

I think that insisting on a change log within the source file is just adding unnecessary friction to the machine and defeats one of the purposes of implementing a version control system in the first place.

Version control can replace those change log comments in the code if the devs on your team are using it right.

If your team aren't adding comments on check-in, or are leaving unhelpful comments, then it will be pretty hard to find the information you are looking for in the future.

At my current company, we are required to submit a comment with every check-in. Not only that, but we are expected to attach each check in with a ticket in Jira. When you look in Jira in the future, you can see every file that had been checked in and when for that issue along with the comment that was left. It's pretty handy.

Basically, version control is only a tool, it's how your team uses that tool that will provide the advantages you are looking for. Everyone on the team needs to agree with how they will use it to best provide bug fix tracking as well as clean code revisions.

This is a leftover from the days when VCS logs were confusing and VCS systems were difficult to handle (I remember those days, somewhere at the backend of the 80's).

Your suspicion is entirely correct, these comments are more of a hindrance than a help and any modern VCS will allow you to find exactly what you are looking for. Of course, you (and your colleagues) will have to spend approx. 30-60 minutes learning how to drive all these features (which, I suspect, is actually the reason why these comments are still there).

I keep it (almost) with George. Comments in code are only really justified if they explain something that isn't immediately visible in the code itself. And that happens only rarely in good code. If you have the need to have plenty of comments in your code, that's a smell of its own and it shouts "refactor me!".

We do still include them in stored procedure source because that's how we can tell exactly which version is in place at a client. The rest of the application is distributed compiled, so we have module version numbers that link back to source, but the SPs are distributed as source to the clients for local in-the-database compilation.

Our legacy PowerBuilder code still uses them, but I think that's just as a comfort factor for certain peeps. That also gets compiled for distribution, so (IMO they oughtta) use the friggin VCS history.

+1 I was going to write this answer but you've saved me the trouble. Not just stored procedures are distributed as source, but many scripting languages too. I use OpenACS and TCL a lot - often if a client has a forked version of a particular file, the only way to be GUARANTEED that you know which version they have is by reading the change log. Particularly handy if you are logged into a client's site and can't easily do diffs with the version control software.
–
TrojanNameJun 14 '11 at 19:42

Change log comments are exceptionally helpful in code when they help a subsequent developer ask better questions regarding new requirements. When a developer sees a comment, for example, that Fred made the Foo field required 6 months ago in order to satisfy some requirement, that developer knows that they need to ask some question before implementing the latest request to make Foo optional. When you're dealing with complex systems, different stakeholders are likely to have different priorities and different desires. Change log comments can be very helpful for documenting which of these trade-offs have been made to avoid problems in the future.

Now, if every developer checked the complete history of every line of code before making any change, having these comments in the code would be redundant. But this is very unrealistic both from a workflow standpoint-- most developers would just change the validation without researching the history of who added it and why-- and from a technology standpoint-- a version-control system would have difficulty tracking the "same" line of code if it has moved from one line to another or been refactored into a different class. Comments in the code are a much more likely to be seen and, more importantly, to prompt a subsequent developer to note that a seemingly simple change may not be so simple.

That being said, change log comments in code should be relatively rare. I'm not advocating that change log comments be added every time a bit of code is refactored or when a true bug is fixed. But if the original requirement was "Foo is optional" and someone comes along and changes the requirement to "Foo is required in order to support downstream process Bar" that is something that I would strongly consider adding a comment for. Because there is a strong possibility that some future user/ analyst is going to be unaware of the downstream Bar process and unaware of the reason that Foo was made required and will ask to make Foo optional again and cause headaches in the Bar process.

And this is before considering that organizations may decide to change their version control system with some frequency particularly as they grow from small companies with a handful of developers to much larger companies. These migrations will often result in the loss of change log comments-- only the comments in code would be preserved.

Are there any VCS conversions that don't preserve commit messages?
–
David ThornleyJun 14 '11 at 19:47

@David - I've seen plenty that didn't. I don't know whether there were technical obstacles to doing so or whether someone decided that it was easier to just "start fresh" and check in all the code to the new VCS on day 1.
–
Justin CaveJun 14 '11 at 19:51

adding a comment explaining why a process changed can be useful, and sometimes adding a comment explaining when it changed, but I don't see the merit in putting that comment in the form of a change log. For one, it somewhat disguises the utility of the comment ("Oh, it's just a change log comment"), and two, it encourages further, unnecessary change log comments (reinforcing #1).
–
Ben HockingJun 15 '11 at 0:26

Quit using visual source safe? All useful modern source control systems handle change logs properly. We know this by definition because if they didn't, they wouldn't be considered useful. Spend 30 minutes learning to use the source control you have, it'll make you a lot more effective than just praying someone who knows your tooling will spare you some crumbs. Not to mention, quite often code history is irrelevant, you're only testing and writing now, now.
–
ebyrobNov 7 '14 at 16:06

As to "changing source control" - nearly all modern systems can move history one to the other, output individual project level change files, or with a small effort actually embed their change-logs into the source file (when it becomes appropriate on a move not before). So the technology is there, people choosing not to use source control properly is the problem.
–
ebyrobNov 7 '14 at 16:20

I'm surprised to see no-one mentioned this, but isn't the original reason for this to comply with license requirements? That is, some licenses say that any change you make to the file must be noted in the file itself?

I worked at a place where the Sarbanes Oxley compliance that they believed in demanded change blocks in source files. They also used PVCS (a.k.a "Dimensions") over the course of years, so they really had no version control whatsoever, but what did I know?
–
Bruce EdigerJun 14 '11 at 21:37

@Marco: I don't remember or I would have linked to it.
–
Sverre RabbelierJun 19 '11 at 0:14

1

Section 2a of the GPLv2 requires it. Most projects ignore it, some have a single "ChangeLog" file (often generated from their VCS).
–
dsasMar 27 '12 at 22:17

The reason that we continue to maintain a change log in our comments section is for ease of use. Often when debugging a problem it's easier to scroll to the top of the file and read the change log than it is to open up the source control file, locate the file within it, and find the changes made

Personally, I find it a bit of a pain when a 200 line file becomes 1,000 lines long due to adding a changelog into every source file. This extra noise actually obscures what is important before long at all.
–
ebyrobNov 7 '14 at 16:14