Code Review Tip: Using the ‘warning’ Preprocessor Directive
Using C#'s #warning preprocessor directive to flag remote code review feedback so it shows up as a build warning instead of getting lost in email.
Communicating changes back to a source developer during a remote code review (when the review is done while the developer is not near) can be very difficult. Emailing the thoughts/changes/errors is an option, but this option has “no teeth”. These items are likely to get missed and most likely will not get addressed. My preferred method is to use the #warning preprocessor directive in C#.
The Warning Preprocessor Directive
I’ll add a warning directive with some text defining what the issue is. I’ll then submit the code and then re-assign the feature ticket to the developer who’s code I reviewed.
Here’s what I’m talking about:
Now the main benefit to this approach is that if I have warnings as errors turned on I will get an error during compilation. If I do not, I’ll get a warning. My build system reports the warnings as well as errors so I’m able to keep track of what needs to be fixed still.
Here’s a screen shot of the warning that shows up from this code above:
I’m now able to keep track of the warnings/edits and also make the visible via the build system on the build server as well as each developers machine. Collective code ownership, baby.
Other Tips
-
Create a nomenclature.
-
Example: Include the ticket number or check in number that this original fix was in question of. Such as : FIX [ CheckIn-#OfCheckIn] – Comments such as …
-
FIX [CheckIn – 1499]: Don’t hardcode UID/PW Combos …
-
Have your build break on errors. This will cause you to fail fast.
-
If the offense is a system halting issue, use the error preprocessor directive. This works the same as the warning, but throws a compilation error. Fast Failure, baby.
