Code reviews are an
important part of any project's health. They are able to find and
correct code defects before making it into the product and thus spare
everyone the pain of having to find them and take them out. They are
also relatively cheap. Assuming your team wants to implement code
reviews, it is important to lay out the expectations clearly. Code
reviews can be contentious without some guidelines. What follows are
the rights and responsibilities as I see them in the code review
process.
Participants:
Reviewer - Person (or persons) reviewing the code. It is their job to read the code in question and provide commentary on it.
Reviewee - Person whose code is being reviewed. They are responsible for responding to the review and making necessary corrections.
Must Fix Issues:
A reviewer may comment on many aspects of the code under review. Some of the comments will require the reviewee to change his code. Others will be merely recommendations. Must fix issues are:
Other items are merely recommendations. The reviewer can comment on them, but the comments are only advisory. The reviewee is under no obligation to fix them. These include:
In my teams there is no ownership of code. Some people touch certain pieces of code most often and may even have written the code initially. That doesn't give them special rights to the code. The person changing the code now does not need to get the initial writer's permission to make a change. He would be a fool not to consult the author or current maintainer because they will have insights that will help make the fix easier/better, but he is under no obligation to act upon their advice.
http://blogs.msdn.com/b/steverowe/archive/2008/08/13/code-review-rights-and-responsibilities.aspx
Participants:
Reviewer - Person (or persons) reviewing the code. It is their job to read the code in question and provide commentary on it.
Reviewee - Person whose code is being reviewed. They are responsible for responding to the review and making necessary corrections.
Must Fix Issues:
A reviewer may comment on many aspects of the code under review. Some of the comments will require the reviewee to change his code. Others will be merely recommendations. Must fix issues are:
- Bugs - The code doesn't do what it was intended to do. It will crash, leak memory, act erroneously, etc.
- Potential maintenance issues - The code is not broken, but is written in such a way that it will be hard to maintain. Examples might be magic numbers, poorly named variables, lack of indirection, lack of appropriate comments, etc.
- Coding standard violations - If the group has a coding standard, it must be followed. Deviations from it must be fixed when pointed out.
Other items are merely recommendations. The reviewer can comment on them, but the comments are only advisory. The reviewee is under no obligation to fix them. These include:
- Architectural recommendations - The reviewer thinks there is a better way to accomplish the goal. Seriously consider changing these, but the reviewee can say no if he disagrees.
- Style issues - The reviewer wouldn't have done it that way. Fascinating.
In my teams there is no ownership of code. Some people touch certain pieces of code most often and may even have written the code initially. That doesn't give them special rights to the code. The person changing the code now does not need to get the initial writer's permission to make a change. He would be a fool not to consult the author or current maintainer because they will have insights that will help make the fix easier/better, but he is under no obligation to act upon their advice.
http://blogs.msdn.com/b/steverowe/archive/2008/08/13/code-review-rights-and-responsibilities.aspx
No hay comentarios.:
Publicar un comentario