rc3.org Strong opinions weakly held

Four reasons to file the bugs you find in code review

In Commentary on 19 January 2010 with 6 comments

I have a habit, some would call it a bad habit, of fixing bugs whenever I see them, usually without ever filing a case in the bug tracking system or telling anyone that I found the bug at all. Usually this happens when I’m reading through code I’m not ostensibly working on. When I see bugs, I fix them. Someone with fewer battle scars than I have may think that’s a great habit and that there’s no need to apologize. Here’s why I should always file a case, even if I fix the bug right then:

  1. Some other developer may unknowingly (or worse, knowingly) depend on behavior that results from the bug. In other words, fixing the bug may break something else.
  2. Your customer may expect or worse depend on the behavior caused by the bug. Bugs that are found in code review are usually issues with business logic. If the bug causes the app to blow up, you rarely need a code review to find it. Customers deal with the interface they’re given, so business logic or formatting issues that look like bugs internally can become part of the de facto contract you have with them. Making changes without telling anyone can create unpleasant surprises.
  3. In fixing the bug, you may break something else. Any time you edit code, you may introduce a defect. Heck, some developers are as likely to introduce a defect as not. If you change code without telling anyone, you may fix one bug but introduce one that’s even worse, or you may not really fix the bug.
  4. Your testers need to know what to test. Your changes may not get tested if you don’t tell anyone what was broken or why and how you fixed it.

Update: Be sure to check out frequent commenter Stan Taylor’s follow up post on this topic.


6 Comments

While not as important as most of what’s on your list, filing that bug also provides visibility to management/clients about what you as a developer do/are doing. Never hurts to have that, but this is especially the case if your managers are non-technical doofuses (Not that this ever happens) or you are billing a client hourly.

Posted by zack on 19 January 2010 @ 10pm

To expand on the last point, “every bug deserves a test”; that is, it is usually the case that a bug reflects a type of failure that is not obvious and could easily be repeated, otherwise it wouldn’t be a bug. So put a test in to catch it (or if it’s a UI bug, put a test in the test plan for it).

I’m not very good at this kind of thing either though.

Posted by Jacob Davies on 19 January 2010 @ 11pm

These are all about communication, and the assumption that filing a bug is a good way to communicate with developers, testers, and customers (and as zack adds, managers). I guess it depends on the local culture, but in my experience, developers only look at a bug report if it’s assigned to them. The revision control system is a better way to see what’s happened recently. There will always be a check-in when a bug is fixed, but other check-ins are relevant to everyone you mentioned and could just as easily cause any of the same problems. Will you also file a bug report when you check in new code that doesn’t fix a bug?

Posted by Seth on 20 January 2010 @ 2am

Generally all the code we check in is associated with an item in the issue tracker, either a feature request or a bug.

Posted by Rafe on 20 January 2010 @ 2am

Great post! These are very good reasons for submitting bugs during code review. Too often the issues found during code review get lost in the shuffle.

Atlassian has tools for both bug tracking and code review, and we’ve integrated them such that you can create bugs from code reviews with just a few clicks: http://www.atlassian.com/software/crucible/tour/jira-integration.jsp

Posted by Jesse Gibbs (Atlassian Employee) on 20 January 2010 @ 1pm

@Seth – As a QA engineer, my nirvana is a system that shows work item IDs (defect, requirement) completed with each commit (or build) as well as a list of files changed with diffs (Let’s not get bogged down here in the discussion of whether EVERY commit must have a work item). That provides both angles: the documentation (defect) angle and the ability to see what exactly changed.

Having the ability to look at diffs (preferably with an indication of why something changed, hence associating work item IDs with commits) serves several purposes: 1.) it allows me an easy way to familiarize myself with the code, 2.) it helps me decide how I need to test something, and 3.) it allows me to spot unreported changes and, at the very least, ask the developer why he changed it.

Posted by Stan Taylor on 20 January 2010 @ 2pm

Leave a Comment

← →

About

This is the weblog of Rafe Colburn. I'm a software developer and computer book author, but this blog is really about whatever catches my interest on a day to day basis. I've been blogging since December, 1998.

Recently

Et Cetera

You should follow rc3.org on Twitter here. If you know what an RSS feed is, you can subscribe to the rc3.org feed here.


← Back to Home

© rc3.org. Powered by WordPress using the DePo Skinny Theme.