Should the commit history be used to convey critical information to developers?

Question

During a meeting regarding the rollback of a third-party SDK from the latest version it was noted that the our developers already flagged in the commit history that the latest version should not be used.

Some developers argued that this was a bad practice and it should have instead been noted either in the source file (i.e. // Don't upgrade SDK Version x.y.z, see ticket 1234) or in a project level README file. Others argued that since the commit history is part of the project documentation it is an acceptable location for such information since we should all be reading it anyway.

Should the commit history be used to convey critical information to other developers or should such information be duplicated to another location such as a project README or comments in the relevant source file?

Answer 1

If I was going to look at upgrading to a newer version of a third party SDK, the last place I'd look is in the history of the source control system.

If your product is using version 2.0 of an SDK and someone is interested in upgrading to 3.0, I don't think it's reasonable to think that they should look backwards in time in your source control system to find out that it's not a good idea.

Here, we have a team wiki that has a handful of pages with interesting info that every developer reads (coding conventions, how to set up a development environment to build the product, what third party stuff you need to install, etc). This is the sort of place that would be appropriate for a warning against upgrading a third party library.

Answer 2

It should be noted in the commit history but the absolute best place to put the notice is in the same place you define the dependency. If you have for example a maven .pom file that declares your artifact dependencies, I'd do something like:

<!-- Do not change the SDK version because it causes Foo crashes. For more detail see Issue #123 -->

Directly above your <dependency> line.

Issue #123 would include details about how it crashes, the version you updated to that caused the crashes, and it probably should then get added to your backlog again for revisiting later - it's possible that there's a yet again newer version that fixes the issue. Either automatically by editing the ticket, or manually yourself, it will email the team to let them know about the current issue now, and by being in the tracker it lets people find it again later.

The reason to put it with the dependency declaration is because whoever wants to change the version will see it at the moment they want to change it and understand why they shouldn't.

I wouldn't comment in the source code because I can easily picture a situation where someone runs a check on your dependencies and starts upgrading them. They should not need to scour the codebase for every TODO comment to do that.

Linking to the issue ticket lets a curious developer know how it failed and potentially revisit it later. Without that, it might become fairly static and never be updated again.

Answer 3

Critical and non-intuitive pieces of information should be documented where people will be looking when considering the information.

For the teams and projects I've worked on, I would commit the roll-back with the comment about why the new version failed. I would add a backlog story to re-try the upgrade if the new version get's fixed. I would add comments to the build system/build scripts where the library is linked.

The roll-back will provide future developers with context when looking into the project's history. The backlog story keeps the need for this upgrade as an active part of the project. The build system comments are right where the changes will need to be when the library is finally updated.

I would not comment it in the code and I would not add it to a README. Developers thinking about trying the upgrade will not be looking at these pieces. If you do add it there, then when the issue with the library is eventually fixed and an upgrade is done, you will need to remove it. This step is often forgotten: resulting in notes that are counter productive to the project.

---

If your project has a different setup or a different flow, then your answer may be different. I think the key is to put the information right were the developer will see it when doing the work for the upgrade. That way, if the time is not right for the upgrade, the developer will see it and stop, and when the time is right the developer will see it and remove the note so it won't confuse future developers.

Answer 4

I wanted to give Matthew's comment more attention by highlighting his important idea in an answer. There's a reason why you don't want to upgrade your SDK, and that reason should be captured in a unit test. Not a check for a revision number, but the actual underlying reason.

For example, say there's a bug in the new version. Write a unit test that checks for that bug. If they later fix that bug in the SDK, then the upgrade will occur smoothly. If there's an incompatible API change, write a test that checks that either your code supports the new API or the SDK supports the old API. That's more of an integration test than a unit test, but should still be doable.

My company generates 50+ commits per day, and we're not exactly huge. Even if every developer read every single commit message, which let's be frank they don't, the whole reason we need a recorded commit history is because people can't remember it. And people don't go back and read the history later unless there's a problem. And they have no reason to suspect a problem on an upgrade that as far as they know hasn't occurred yet.

By all means send an email to prevent duplication of work in the near term, and note it in your build scripts and a README or errata. However, especially if the problem with the new version is subtle and time consuming to troubleshoot, you need a way of making it obvious. That means a unit test.

Answer 5

I'm recasting the question as "Should I communicate critical information I discover to the rest of the team only via commit message?" Because I think that makes it obvious that no, you shouldn't. I try hard to communicate a lot (this is something that most development teams, in my experience, need to put active effort into) and I certainly do everything I can to avoid creating pitfalls or letting them lie.

If the chain of actions leading me to such a discovery were triggered by a ticket, I'd update the ticket (and make sure the people who should know this have visibility), I'd probably mention it face-to-face (hoping to at least leave someone with some nagging sense that "Gee, I think Damon said something about updating that"), and I'd of course leave a comment in the code at the point the SDK was included so that no one could possibly update it without having a chance to see it. I might see if I could jam it in somewhere on our dev wiki, too, although that's done more with an eye toward future hires, not the current team.

It only takes a couple more minutes compared to the time it likely took to encounter and discover the problem. I certainly wouldn't decide that one of the least-used, low-signal pieces of our documentation and leave it at that.

Answer 6

It should be in the commit history but it should not only be in the commit history, imagine for a moment you hire a new developer. Do you expect that new developer to read every commit message for the past 10 years of your project because a couple of them will be critical to understanding your code base?

Second say the situation but not the code changes, are you going to make "documentation" commits so that you can add commit messages along the lines of "commit message from revision 5432 is now incorrect, here is the current situation."

Answer 7

I'm not sure how your team communicates but I believe the most effective way to say this is to first send and email to the team's email group, marked as "URGENT" with the body saying

Guys, we can't use SDK v x.y.z because it causes the Foo buffer to overflow and the Bar service will crash. Stick with version x.y.y

That's what we've done here and it's the most reliable way to get the message out there. If you really want to be fussy (and if your email system allows it), request a "read receipt" on the email.

Once you've told the whole team, more detailed documentation should be put into a team wiki. This will vary, depending on how you structure your documentation. If you have a section specifically for your applications Dependencies and Requirements, that would be a good place to add it.

An additional place to document this sort of problem might be in the source code itself, although that might not always work. If SDK version ... is only referenced in one or two obvious places, you could include a note about not upgrading.

The file history in source control can be very long, and depending on the developers, could have several entries per day. Someone who's been out on vacation for a week might not have time to read a week's worth of commit history. The README file is a better place as it's a little more central and people might be more inclined to read it, but you can't ensure that everyone will actually read the README. Well, I suppose they might if they see it's been changed...

Answer 8

Something like this should have been put in the commit comments, but it will benefit from being in other places as well.

Whoever makes the decision to upgrade, needs to have the facts. That person may not live in source control. What if someone would have read about this problem on SO and never put it into the code base?

There needs to be some sort of document about this third-party SDK.

• What problem does it solve?
• Why was this particular one chosen?
• What considerations need to be made as far as: versions, upgrades, testing, etc.
• Who makes this decision?

You have a case where something like this made its way into the version control, and you should recommend everyone use this information as much as possible. Only your team can decide where someone is going to do a search in any documentation, source control, or bug tracker to get as much information as possible on the subject. Otherwise, you'll forget, someone will do it anyway, and you'll be lucky if it jogs someone's memory and quickly roll it back again.

Answer 9

I interpret this situation as having two basic problems, possibly three.

• An unwanted SDK upgrade made it into the source, where it could negatively affect the product.
• From the question: the contributor who performed the unwanted upgrade didn't know about a previous, specific decision not to upgrade.

The first of these, in my opinion, is the most serious. If an unwanted SDK upgrade can make it into the code, so can other problems.

Someone suggested adding a unit test case that will fail if it detects the upgrade. While it would prevent the upgrade from occurring, I believe this is a dangerous path, leading to lava flow over time. It seems inevitable that at some point in the future the SDK will be upgraded, to bring in new features or bugfixes, or because the old version is no longer supported. Imagine the head-scratching, maybe even arguments, that will occur when such a unit test then fails.

I think the most general solution is to adjust the development process. For git, use the pull request process. For Subversion and older tools, use branches and diff. But have some process that allows the senior developers to catch these kinds of issues before they make it into the codebase and affect other developers.

If the pull request process had been used in your situation, and if each pull request is narrow and specific, not much time would have been wasted. A pull request to upgrade the SDK would have been submitted, and declined with comment that the upgrade is not wanted. No one else would have been impacted, and there would be no need now to revert the SDK upgrade.

But to directly answer the original question, I agree with others that expecting all developers to fully read the entire revision history of the code, release notes, etc. for notices like this is a waste of valuable time. What's wrong with a short team e-mail?

Possible third issue: Why is the upgrade not wanted in the first place? Clearly at least one developer thought the upgrade would be a good thing. There are many good reasons to delay an upgrade, but also many bad ones. Take care to avoid the lava flow (unnecessary backwards-compatibility code) and cargo cult ("we can't upgrade that, but I don't know why") anti-patterns!

Answer 10

History is a great place to put data intended for a reader who is consciously looking for it, and has a general sense of where it should be. It is a very bad place to put data which must be proffered to a user, rather than searched for.

Histories are very large bodies of relatively unsorted text. They are usually intended to provide a developer with detailed information about what changed, and why it was changed. This can be information overload unless one knows what they are looking for.

If a user does not know what they are looking for, then the information is quickly buried under hundreds of commit logs, and they have no tool to prune down the pile of information in front of them.

Answer 11

I would say adding that type of information to a commit history is ok but it still needs to be documented properly. We recently started using confluence (by atlassian). Its searchable, you can set certain pages as favorites, etc.

Some other tools might be a public notebook in evernote or google docs.

Answer 12

Expanding on Karl's answer, I would go with an approach that automatically enforces the restriction as part of the checkin process itself. You need something that does not require any proactive action on behalf of the developer, such as reading a doc/wiki/README, and can not be overridden covertly.

In TFS source control land you can code custom checkin policies that execute rules on checkin. For example, you could define a policy that evaluates the version in a config file with a pending checkin and will fail if it does not equal x.y.z. These rules actually prevent the developer from performing the checkin and can provide a descriptive message. The policies can be overridden, but it is possible to generate alerting when this happens.

An alternative could be gated-checkins that fail will with some form of unit test that either directly or indirectly evaluates the SDK version, as Karl mentioned.

I appreciate this answer is very TFS-centric, but possibly similar features exist in the version control/CI systems that apply in your situation (if not TFS).

Answer 13

In contrast to the accepted answer, I insist commit messages are a valuable(primary) source of information.

Let's role-play a release coordinator. You are responsible for updating the current PROD version thus winning or losing a huge amount of money for the company. This is a speculation: I have never been in the position.

You check your Jira, Confluence, Artifactory, sales metrics etc. But you also check the last merge commits. You grep in your mind I could have broken X, improved Y...didn't break it(yeah, right)