Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

During a meeting regarding the rollback of a third-party SDK from the latest version it was noted that the our had 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 there should have been something 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?

share|improve this question
53  
It sounds like your project has pretty serious communication problems. –  tzerb Jul 29 at 14:08
70  
Do you require new hires to go through the entire commit history? –  Steve Evers Jul 29 at 15:49
3  
New hires should not be rolling through the code base and changing dependencies without specific direction to do so. –  Midnotion Jul 29 at 16:23
27  
@Midnotion So somewhere on the path from new hire to main developer you take time to go through the entire commit history? Fascinating. –  Nathan Cooper Jul 29 at 16:48
5  
That still doesn't make it a good idea to put critical information solely in the commit history. –  17 of 26 Jul 29 at 18:21

12 Answers 12

up vote 124 down vote accepted

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.

share|improve this answer
48  
Indeed, the commit history is the history of the project. Just like it would be stupid to have crucial information in the history of wiki edits, and not on the actual page itself. History (whether code or wiki) is for reference on what happened previously, not on what should happen now. –  Ordous Jul 29 at 16:38
38  
If possible for the fictional SDK, I would add a unit test designed specifically to pass under V2 and fail under V3 with an explicit assert statement giving the reason not to upgrade to V3. The commit history is a good place to state why you're making that one change for code reviewers, not for documenting best practice. –  Klors Jul 29 at 22:58
3  
@Klors As long as you don't restrict yourself to the most pedantic definitions of unit tests and refuse to do other types of automated tests, a test based on checking the file system/etc for the libraries name (if it has the version encoded in it) or a hash/checksum of the file itself could be used to throw a red flag at anyone wanting to update the library even in cases where the new versions flaw is difficult/impossible to capture in a test. (eg multi-threaded race conditions) –  Dan Neely Jul 30 at 15:18
12  
@Klors I was thinking the same thing, but in my opinion, the unit test should demonstrate the reason for using v2 over v3, that way if the unit test passes with v4, you don't have a unit test that requires v2 for no reason. –  Matthew Jul 30 at 19:20
9  
Another reason not to use the commit history for this: the commit history is a permanent record. If the reason for not upgrading goes away, the commit history is still going to contain the warning not to upgrade, which could be confusing. You need somewhere where lists of requirements like this are kept up-to-date, so that developers don't have to double check whether things are still relevant. –  Jules Jul 31 at 7:16

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.

share|improve this answer
11  
I agree: the best location for critical information is "in as many places as possible." Maybe the pom.xml or equivalent project file, a readme, etc. but the commit history is still good. If I am looking at upgrading a library I may check the history of the existing version to see when it was updated as well as any notes about issues the committer had. But I would not want to go digging through the history to gather all the documentation. It is a good supplementary source. –  Snowman Jul 30 at 22:18

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.

share|improve this answer
7  
Yes, the comment needs to be placed "in the path" of the change. So it's technically impossible to do the action without seeing the warning. It's a lot like safety controls. –  dss539 Jul 29 at 19:40
    
+1 for the suggestion to create a story/ticket for the upgrade in your issue tracker, set to "On hold" with an explanation of why it can't be done yet. The issue tracker is one place you really can reasonably require people to look before working on something. –  Andrew Spencer Jul 30 at 9:57
    
+1 Quoting Jeff Attwood (although he talks about, ugh, "users"): "The next time you're designing [X], consider [client] myopia. You might be surprised just how myopic your [clients] can be. Think long and hard about placing things directly in front of them, where they are not just visible, but unavoidable. Otherwise they might not be seen at all." blog.codinghorror.com/treating-user-myopia –  heltonbiker Jul 31 at 18:48

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.

share|improve this answer
4  
+1 The real keyword is only. It certainly does not harm if the info is stored in a commit message in addition to other, more suitable places. It did hit the OP, since the commit log was the only documentation available. –  JensG Jul 31 at 10:32

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

share|improve this answer
3  
I like the email group idea. Too many places I've worked rely on individual addresses and things are not passed on to new members. –  JeffO Jul 29 at 14:39
17  
What happens if somebody new comes to your team? Who is responsible for giving them this kind of pseudo-institutional knowledge? –  ABMagil Jul 29 at 15:20
3  
@ABMagil: The information goes into a wiki. Developers who are new to the team are usually started of there, on some introductory pages. Then they come to specific individuals (who have made time available for helping the new dev) when they have specific questions (and they always do). For us, it would probably end up in the "Developer Setup Guide for ApplicationX" as NOTE: Do not use SDK vers. x.y.z because... but the initial communication should be an email. –  FrustratedWithFormsDesigner Jul 29 at 15:28
14  
-1 emails don't work well as documentation –  BlueRaja - Danny Pflughoeft Jul 29 at 19:14
6  
@BlueRaja-DannyPflughoeft: Email provides a simple and easy-to-use method to let everyone on the team know immediately that a problem has been discovered in using a specific version of a specific library and that this version should not be used. As stated, long-term documentation is best done in a team's wiki or some other sort of knowledge base. –  FrustratedWithFormsDesigner Jul 29 at 19:18

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

share|improve this answer

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.

share|improve this answer

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.

share|improve this answer

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.

share|improve this answer
    
this reads more like a comment than an answer. See: How to Answer –  gnat Jul 31 at 0:41
    
Good point, I expanded it. Hopefully this meets StackExchange's criteria better. –  Cort Ammon Jul 31 at 22:07
    
I think this answer best addresses the topic of the question. Commit history is good for saving info if a person knows they should be looking for a note. If there is no reason to examine the 'blame' for a given line, such as the inclusion of an SDK, then that documentation won't be read. –  Seth Battin Aug 2 at 16:53

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!

share|improve this answer
    
While the SDK upgrade, which was a minor version number as opposed to major, was what ultimately lead to this question, this line of reasoning has been popping up in the group for awhile now. –  rjzii Jul 31 at 20:02
    
Why would the pull request have been declined? How is the person responsible for that decision supposed to discover (or remember) the version restriction? –  Ben Voigt Jul 31 at 21:39
    
@BenVoigt Well, either the team leads knew about the restriction, or no one remembered it and this was rediscovered after encountering problems. Either way, using pull requests, there is no blaming the lowly new hire developer, as the seniors would approve the change before it's allowed in. –  wberry Jul 31 at 21:54
    
@wberry: Or a different senior developer processed the pull request from the one who knew about the version restriction. Unless all pull requests have to be approved by all developers, which seems like a dubious expenditure of resources. –  Ben Voigt Jul 31 at 22:02

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.

share|improve this answer

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

share|improve this answer

protected by gnat Aug 1 at 5:43

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?

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