I have been doing a lot of code review lately, and I am unsure of the positive and negative effects and professionalism of putting positive and/or funny comments in code reviews.
We use Github as our code review platform on my team, so the comments are viewable by anyone. I generally try to use this platform so the entire process from start to finish is visible and historical.
It's important to highlight positives as well as negatives. I know if I were reviewing the refactor of a particular hellish subsystem into something neat and clean, I'd probably buy the programmer a pizza for his efforts.
If you're using reviews as training, it's doubly important - highlighting a good piece of code will be helpful for the junior programmers also reviewing that code. They will have a chance to ask questions about why a particular approach or technique is better than another.
Funny: Save it for the water cooler, except in minimal doses - having a prune face is not a requirement for reviewing code.
Positive: Certainly. Review includes both positive and negative/constructive, by definition.
Positive Feedback helps everyone:
For the one receiving the thumbs up, you reinforce their confidence and inspire them to do more of the same through your positive feedback.
For the rest, as others have mentioned, they will learn what to do, as well as what not to do. They will also be encouraged to excel, so they too can one day be in the spotlight.
I once worked for a boss who was liberal in his positive feedback - the team was very successful and productive as a result. He moved on, and others took over that lacked his ability to praise a job well done. Productivity and morale took a nosedive, and many of the better team members left the firm.
I'd say keep the comments clean and to the point, exactly because of culture.
You cannot avoid that some people will take things the wrong way.
In order to mitigate that, a personal one to one talk will smooth things along, if face to face is not possible, chat or email or skype.
Treat comments as a tool of management
Inserting comments into a code review is a form of management. As such, it should be approached as a tool of management.
Use managerial practices when commenting
There is a structure to managing people where the goal is to meet a desired outcome. Some of the main approaches to management will not apply in comments but most will. The applicable topics include environment, leadership, organizing, and controlling.
Environment
Culture
The environment dictates the style of management. The culture and environment of the workplace should be kept in mind when using any tools of management. Usually this is affected by the industry and the size of the company or entity being managed.
Style
If there is a lighthearted culture then that may come across in the style of management being used. If there are very strict guidelines, policies, and consequences then that should be reflected in the style used. So if everyone is on board for a star wars joke referencing droids and a weak minded stormtrooper then a comedic interjection may be applicable. However, if there are dire consequences of not taking the end result seriously then it may need to be avoided.
Leadership
Fundamentals
There are three main pillars of leadership to consider when commenting. Namely they are vision, communication, and judgement.
Vision
It is important when to keep the grand vision in mind when explaining or giving instructions. In comments, this could mean pointing out how small changes affect the project as a whole, what the implications are of taking different approaches, or a tip of the hat to separation of concerns.
Communication
Being a good communicator is important in many aspects of life. It is no different in comments. Employing a wise level of brevity is important - especially since comments shouldn't take much space. Get to the point early, and then back it up with an example if necessary. In a larger organization, this may also include the need to send out a communique or memo if the issue is not localized to one review session.
Judgement
It is important that strategy is used when judging whether or not comments need to be made, and what the changes need to be. Your judgement does not always need to be correct, but it does need to be consistently correct especially when large judgement calls are made.
Organizing
From a managerial standpoint, organizing will refer to keeping the end goal in mind and ensuring that processes are aligned to follow a set of rules. Comments should also keep this in mind in that the comments should when possible build on each other to make sure that the flow of design is followed. It will also be important to keep in mind the scope of the code being reviewed in order to reduce coupling and follow the overall design.
Controlling
Controlling the actions of those being managed is a delicate process. While being firm it should also be kept in mind that the people are important. There are several managerial skills to use while controlling others. These skills are political, conceptual, interpersonal, diagnostic, and technical.
Political
Politics can be found any time there is interaction between people. It is a huge topic, but in a general sense politics revolves around influence. It is important to keep in mind the personal and professional politics at work when making a comment. This could be related to an instruction, a joke, or even a question.
Conceptual
Managing through conceptualization is an important tool. It requires a complex analysis of the situation at hand. When commenting it can be beneficial to include some of the analysis used to come up with the conclusion or change indicated in the review.
Interpersonal
Interpersonal skills are very important when managing. This is also a large topic. Some of the important things to consider with interpersonal skills are mentoring, constructive criticism, and "harpooning".
Mentoring
It is important that management is seen more as a mentor than as an antagonist. In a code review, this means that sometimes it will be beneficial to include a nod to a design pattern or approach which could be used to improve a situation.
Constructive Criticism
Criticism is important because it invokes reflection. However, criticism should be kept as positive as possible when it can be. This means offering valid evidence to support the criticism, and also ensuring that the tone used is not negative. When reviewing code this could include showing an exception or possible scenario which would generate an error while implying a solution instead of showing every place which was incorrect when the whole piece of code would need to be replaced anyway.
"Harpooning"
"Harpooning" is when you figuratively harpoon someone to the ground. This is done by breaking them down step by step without any reprieve until the point that they feel unable to get up. If you harpoon a person in a code review or elsewhere you will lose their cooperation. It is important to avoid breaking someone down excessively.
Executive Summary
Treat comments in a code review as a managerial tool. Keep in mind that comments should be brief, to the point, and constructive. Also ensure that while commenting the person being reviewed is taken into consideration.
Code reviews are a tool for improving code quality, in part by uncovering defects. More importantly, you want to instill good coding practices.
From this perspective, it is important to comment on things done well. In a training context, improvements should also be commented on. If your culture is one of continuous improvement, then you should always be commenting on improvements.
Mistakes, bugs, and bad coding will happen. Point them out in a non-personal manner, and treat them as expected.
From a behavioral modification perspective, reward is much better at producing changes than punishment. I would consider having good work noticed as a reward.
