https://pixabay.com/vectors/code-programming-head-computer-2858768/ |
Once in a while I run into developers who think that peer review can be completely automated by using a good static analysis (generically "lint" or compiler warnings). In other words, run PC-LINT (or whatever), and when you have no warnings peer review is done.
Nope.
But the reality has some nuance, so here's how I see it.
There are two critical aspects to style:
(1) coding style for compilers (will the compiler generate the code you're expecting)
(2) coding style for humans (can a human read the code)
For #2, capabilities vary widely, and no automated tool can evaluate many aspects of good human-centric coding style. (Can they use heuristics to help with #1? Sure. Can they replace a human? Not anytime soon.)
My peer review checklist template has a number of items that fall into the #1 bin. The reason is that it is common for embedded software teams to not use static analysis at all, or to use inadequate settings. So the basics are there. As they become more sophisticated at static analysis, they should delete the automated checks (subsuming them into item #0 -- has static analysis been done?). Then they should add additional items they've found from experience are relevant to them to re-fill the list to a couple dozen total items.
Summarizing: static analysis tools don't automate peer reviews. They automate a useful piece of them if you are warning-free, but they are no substitute for human judgement about whether your code is understandable and likely to meet its requirements.
Pointers:
- Coding style for Compilers
- Coding style for Humans
- Peer review tutorial
- Peer review checklist template
* Note: in teaching I require these gcc flags for student projects:
-Werror -Wextra -Wall -Wfloat-equal -Wconversion -Wparentheses -pedantic -Wunused-parameter -Wunused-variable -Wreturn-type -Wunused-function -Wredundant-decls -Wreturn-type -Wunused-value -Wswitch-default -Wuninitialized -Winit-self
-Werror -Wextra -Wall -Wfloat-equal -Wconversion -Wparentheses -pedantic -Wunused-parameter -Wunused-variable -Wreturn-type -Wunused-function -Wredundant-decls -Wreturn-type -Wunused-value -Wswitch-default -Wuninitialized -Winit-self
No comments:
Post a Comment
Please send me your comments. I read all of them, and I appreciate them. To control spam I manually approve comments before they show up. It might take a while to respond. I appreciate generic "I like this post" comments, but I don't publish non-substantive comments like that.
If you prefer, or want a personal response, you can send e-mail to comments@koopman.us.
If you want a personal response please make sure to include your e-mail reply address. Thanks!