What's an effective strategy to mark code as reviewed?

Question

I am reviewing a big C# code base and thought on how to mark it as reviewed when it is, in order to not review it twice.

Currently I have a ReviewedAttribute which I use to decorate members that have been reviewed:

public class ReviewedAttribute : Attribute
{
}

That approach however, misses an important aspect:

• How to find code that hasn't been reviewed yet ?

Easy you'll tell me, a code block that doesn't have it hasn't been reviewed ! The problem with this approach is that you'd have to open each file to see if the attribute is in it ...

I could also add a NotReviewed attribute on each place that hasn't been reviewed yet but it's an extremely tedious approach as well ...

Basically I'm looking for an approach like XML documentation does when it's enabled : a warning issued for each place that hasn't been documented.

Since if I haven't found such a way, I'm starting to think this is not the best approach.

Question:

Is there an effective strategy to mark blocks of code as reviewed ?

Answer 1

You could use the #warning directive to alert you to code that still needs to be reviewed, but the downside is that you need to mark every file with this. Fortunately, it would be pretty easy to write a little script to append a line at the end of each *.cs file in your project.

Other than that, take John Wu's advice and remove it after it's been reviewed. The advantage over a plain comment and grep is you'll get a bit of help from the IDE and compiler. You'll get zero false positives this way.

Note: Append it to the end because the file will still compile if you put it at the end. I don't think it will compile if you put it at the top.

Answer 2

Given that this question pertains to "catch up" reviews, not ongoing reviews (which should be done as a matter of course on checkins/changesets, not files), I would actually invert this problem. I don't care if a code file has been reviewed-- after all, that still doesn't mean I don't need to review it again! Instead, I care if a file has never been reviewed.

So:

1. Add a comment to every single code file that is pending review. The comment could be something like /** Pending baseline code review **/

2. When you review a code file, remove the comment.

3. To find files that still need a review, search for the comment.

4. When all the comments are gone, your initial/baseline code review is complete

Answer 3

I would simply use a comment having a special marker string that is unlikely to occur elsewhere indicating the entire file has been reviewed, and then grep as the tool to find non-reviewed files. However, if you're using Windows, then you may lack grep. The grep option -L will print files without match.

And about reviewing files vs commits: you do want to at some point of time switch to the strategy of reviewing every single change to the repository. It's much safer that way, and also less work: if you review a file, somebody changes it, then you need to review it again.