It's important to be open to code review requests but we've all been there. You've got comments on your code from a reviewer and it feels like you might as well rewrite everything from scratch. It's demoralising and feels like a waste of time, especially since you know it works and fulfils the requirements of the task.
Situations like this can lead to you cherry picking your reviewers, having endless back and forths about a forEach vs a for, and generally feeling a bit fed up with the whole process.
Well, let me introduce you to a system that lets the reviewer express their thoughts in a manner that indicates exactly how important each comment is and how much attention you need to pay to it.
MoSCoW, taken from the world of project management is the name given to a list of requirements laid out as:
- Musts
- Shoulds
- Coulds
- Woulds
I won't dwell on what they mean exactly in the world they originate from because it's not relevant and the system is so self explanatory you've probably already worked out where I'm going.
M/S/C/W
Each comment should begin with M:, S:, C or W: and then the comment.
- Must: this has to be changed to be approved. These are errors that either go against the coding standards of the team or are clearly just incorrect and will lead to a bug. These can't be ignored.
- Should: these are things that would be an obvious and clear improvement. You have to give a reason to ignore this and the code reviewer has to agree. A third party can be brought in if needs be.
- Could: these are for niceties that the reviewer thinks would improve the codebase in some way but are probably going above and beyond what the task requires. If they have an idea for a slightly better function name or they want to leave a comment but aren't all that bothered if you take the advice then this is the prefix to use. These can be ignored without a reason given.
- Would: these are reserved for the "I wouldn't have done it this way" comments that require lots of rework but are 100% personal preference. These rarely get used because a would is really just the reviewer being given space to express their thoughts. An ideal usage of this is to educate a junior member of the team and perhaps follow up with a conversation around it post review.
In the example above about a for loop I would say:
S: Use forEach here to avoid off by one errors and aid understanding of
loop conditions
or similar.
Summary
Introduce this at your next team meeting, get a clear understanding of your musts and a rough idea of some shoulds and you will see how liberating this is for everyone involved in the code review process.