Guidelines
How to be a good reviewer
Introduction
Reviewing code submissions should be easy, right? Surely, code is either objectively good or objectively bad and must be judged accordingly.
But it is truly not that simple. Code quality, conventions, performance, flexibility and a variety of other technical considerations are certainly important. Those considerations will be covered, but first and most importantly the human element ought to be addressed.
It is absolutely essential that reviewers consider the human behind the code. Building a healthy and happy community is more important than code quality. Code quality does absolutely have some impact on the community but there are a variety of other non-technical issues to consider, for example:
- Time pressure
- Lack of experience
- Under-performing colleagues
- Mundane or repetitive tasks
- Unexplained and broken code
- Poor decision making
- Imposed limitations and platform constraints
- Lack of monetary compensation
Judge code, not people
When dealing with developers, and people in general, approach is everything. Obviously, we all want high quality code, but there are many ways to persuade or encourage good code and some ways are better than others. One approach might be appropriate for an experienced developer, but would cause tension or problems for a less experienced developer. Firstly, when judging a plugin submission consider the experience of the developer. There are some obvious indications to determine this:
- Is the submission their first submission or do they already have a lot of plugins?
- Does the code itself indicate an experienced developer? e.g. Do they avoid common pitfalls typical of newbies?
- Is the developer responsive to recommendations or did they abandon the submission after review?
As individuals we depend on eachother for feedback and criticism. Criticism itself is not inherently bad, but it is important to frame criticism in such a way that is not excessively critical. For example, it is essential to address specific problems and not the individual who may have created them. The success of the group is dependent on the cohesion of its members. Giving and receiving feedback is necessary to leverage collective intelligence and encourage collaboration.
The review process is inherently collaborative and submissions ought to receive multiple reviews before being approved. That said, reviewers will not always agree with eachother and the developer under review is not necessarily obligated to fulfill every request. Some criticisms are more important than others; it is good to specify which must be addressed and which can be safely ignored.
Keep it professional
Be professional and careful with the language used to criticize the work of others. For example:
- Do not criticize the developer, focus specifically on the code in question.
- Provide third-party references, or code by other developers, not always your own code.
- Do not assume the developer lacks particular knowledge, rather that they just forgot.
- Definitely assume that other developers know things that you do not and can provide you with valuable insight if given an opportunity
When making technical suggestions, please provide links to relevant documentation or even third-party coding resources. Third-party links should be authoritative preferring information from well-established sources such as Microsoft, Wikipedia, or StackOverflow.
Learn the easy way or the hard way, both are valid
The wisdom provided by others can be an invaluable tool to shortcut decision-making by allowing an individual to rely on experts who already have the requisite knowledge. However, often developers will not want to take such shortcuts, rather preferring to gain the experience directly. Both ways of learning are equally valid and reviewers should try to establish the developer's preference in this regard and work around it.
Personality conflicts
As a general rule, engineers tend to be more disagreeable on average. People usually like others to whom they can relate, or people with whom they have had positive interactions with in the past. Reviewers should take that into account when choosing to review a submission. Does a reviewer have a pre-existing relationship with the developer? Is that relationship positive or negative? If negative, perhaps the reviewer ought not to review the submission and leave it to the other reviewers.
Prioritize creativity
Reviewers should be more forgiving with original work. If a particular submission has many alternatives, the abundance of that type of thing should affect the degree of criticism applied to it. In other words, when reviewing a plugin that has few (if any) alternatives, the submission should be judged less harshly simply because of the high premium placed on having something new. Moreover, be sure to check for existing solutions and compare them to the submission.
Conclusion
Please remember that the community depends on these reviews in order to usher in new developers and resources which may provide little value in the moment, but could represent great value in the long-term.
Thank you for considering these guidelines, and happy reviewing.