|
|
|
|
How to Plan and Conduct Effective Code Reviews (a.k.a code walk-throughs) - by - Partha Kuchana
***. No place for opinions
Planner: Largely, individuals create their opinions based on their personal experiences and their perceptions. As individuals are different, so are their experiences and perceptions.
So, it is a very bad idea to do a code-review based on "opinions". It could lead to varying levels of quality-expections with by the same reviewer as his/her opinion varies
based on his/her experiences (and his/her mood?) . Instead, I recommend having a clearly defined set of standards that is used as a checklist while performing a
code review. As a result, no matter, who is conducting a review, the quality-expectations will remain the same.
A friend of mine shared an experience where a reviewer would say... "in my professional opinion..." followed by not so productive suggestion. No matter, how you qualify
the word 'opinion' (professional in this case), there is no place for opinions in a review session. If one has a suggestion to add a new standard, it needs considered to be first added
to the standards document and then applied in a consistent manner.
***. Document and Publish Coding Standard
Planner: While it is highly discouraged to evaluate code based on 'opinion' in the review session, it is recommended to involve all concerned parties in a collaborative
effort to create 'The Coding Standards Document'. It is also important to publish such document and make it readily available to programming resources.
The Standards document must also have place-holders for a reviewer to specify the result of the review (pass/fail) for each individual item and to provide additional information.
***. Prevent the mis-use of the Review Process
Planner: You as a planner needs to make sure that the review process is not mis-used. I interviewed an IT guy who worked for a consulting firm in Boston. He worked on a project, where
the project architect would say that he is going to do a code-review when anyone on his team raises a question on his design/architecture. Sounds silly, but this is one of many forms
the review process could be mis-used.
***. Document Review Results
Reviewer: All reviewers MUST be asked to document their review results. Based on human psychology, individuals take a different (higher) level of care when they have to document something. It
ensures that only productive and no-nonsense feedback reaches a programming resource.
[article continues after the resource listing below]
***. Be Professional
Reviewer: A reviewer MUST remain professional during the review process. A reviewer must understand that it does not automatically make him/her a better professional than the person who
created the artifact being reviewed. It is a fact that there are too many people who are far more efficient/intelligent than the individuals they work for. Belittling someone's
work and making non-productive comments is a clear reflection on a reviewer's professionalism or lack there of. It indicates that he/she needs a vacation asap.
Another resource I interviewed for writing this article shared an experience where a reviewer called the artifact he was reviewing is 'worthless'. The review involved the
the creator of that artifact as well.
*** Be realistic
Reviewer: The world has never stopped because there is an un-used variable left in a program or there is a commented code line.
*** Be realistic
Reviewer: There several different ways to implement a piece of functionality/logic, so get over it, if what you are reviewing is not matching with how you would do things 'in future' when you are
leading a world-saving-mission.
***. Sample Application
Planner: It is recommended to have an application that could be used as a example, for programming resources as they create code. You definitely do not want to have
commented code, when one of the standards is not to have commented code.
***. Rotate Review Team Members
Planner: It is a good idea not to have the same set of resources do code reviews all the time. It is a good idea to pick reviewers from the programming resource pool. Again, based on human
psychology, it forces individuals to sort of 'behave' and not go crazy with the new found reviewer role , when they know that they could find themselves on the other side the next day.
What does it all do at the end? You will get a more desciplined code reviewing process with a clearly defined objective and RESULTS
***. Nothing personal
Developer: While there is a good number of half-baked potatoes in your technology leadership team, it is quite possible that you will find someone who has great skillset and has lots of respect for ya.
So, take any and all feedback in a professional manner and understand that nothing is personal. Be very clear that you MUST follow the standards document. Whenever there is an exception, you must
bring it to the attention of your technology leadership team and get prior approval for not following a specific standard.
|
|
|
