We used to do manual code reviews (i.e. no special tools) and this was the best method we found. Our development department doesn't have team foundation server, so I can't comment on that.
- All work is tied to a bug or an agile story that has a unique ID. When the work is checked in (or shelved, which is submission without actual check in), the description will always have this identifier.
- In a stand-up or over e-mail the reviewer would be notified that bug ### or story ### is ready for review.
- They do source control diff between new versions of the files and previous versions and copy/paste the output of diff into a word document. We have a code review word document template which is preset to use have 2 levels of headings: level 1-module, level 2-file name. It also defines formatting style for actual code (8pt Consolas). This step obviously was the biggest pain, but with Word template and Ctrl-A, Ctrl-C, Ctrl-V sequence, it's not that many clicks.
- Now that the document is in word, the reviewer can highlight the code in question and add comments using Word's review system.
- All documents are stored in a document repository and labeled with corresponding bug or story number.
- Once the document is done, the link to the repository is shared with original developer. Then we'd either have a code review meeting if changes were significant enough and need discussion, or just leave it up to original developer to go through comments on his own time.
After trying numerous manual methods, this was the one we found to have the least amount of overhead while allowing us to actually review every change.
However, our engineering team just rolled out Review Board and although I haven't used it that much yet, so far I'm loving what I have seen. It has all the flexibility of our word docs, but no more copy/pasting and manually fixing formatting. As an added bonus it keeps a permanent archive of all comments so you can go back years in time if you ever need to. Also it allows you do diffs of diffs, which is great when you want to do a review of a code review. This part we found to be very difficult with manual procedures because you can't see what was changed in response to first code review, instead you end up redoing the entire thing.
Although you did say you don't want to use tools, I'd strongly urge you to consider Review Board. It is open source and completely free. So you can roll it out for yourself and the 5 people you maybe working with. The rest of your company doesn't have to use this tool if they don't want to. And you don't need to worry about getting any purchase approvals.
== Update to comments from question: ==
On my team, we have people in NY, CT, TX, Poland and India. What makes things even more interesting is that extremely high percentage of the team doesn't know the product or technology all that well, so very few of us do most of the review. So yeah, senior devs are definitely busy. In the process I outlined, primary reviewer would do initial walk through the code independently on his own schedule. But afterwards we schedule a meeting and primary reviewer walks the coder through his comments. The meeting can have other reviewers, but they are considered secondary and are not obligated (but not discouraged from) to review every file or make comments.
I agree with others comments that having the final meeting in real-time, even if you have to use web conferencing is much better for knowledge transfer and for helping your new guys understand the code so that they'll start producing things that make your senior devs even busier. But again, that depends on the volume and type of comments, sometimes (very infrequently) comments are so minor that the meeting is skipped.
I can also relate to you when you say it's hard to roll out new tools. I work for a very large corporation and typically there's so many people involved that even outside of the fact that no purchases are ever approved, there are so many interests/agendas that nothing is ever agreed on. What's nice about Review Board is that you can skip all that and just start using with your small team and if you have to (if it really comes to that), you can host the web services on your own dev machine.
Best Answer
Short Answer
No.
Yes. According to your suggestions, not instructions. Instructions sound too authoritative.
Use a tool to give the feedback. You may use Visual Studio.
Long Answer
I used to use Visual Studio but I constantly had to ask other developers, "Hey can you send me your work so I can review it?" I did not like that and it never worked out really well. Now, I use Review Assistant because I can start a review by looking at checkins. I do not need to rely on another developer sending me work to review. It also helps me mark items as defects, or simply mark items to be looked at by the author. This works for our team because once we start a review, it stays right there on the review board and does not get lost in translation. This is integrated with Visual Studio. As I mentioned, Visual Studio also has its native review process but I find it has limitations and the process is not natural; therefore, I use Review Assistant.
This tool also helps with the back and forth process, discussions etc.
The process, more or less, is as following:
I review something, then I send it to the author (in your case junior dev). They make changes and then they send it back. I look at the changes and provide feedback. If I am good with the changes, I close the review. Otherwise it goes back and forth. Sometimes if there is too much back and forth, I will just walk over to their desk and use a whiteboard--it really expedites the process.
Code reviews are a sensitive area so be very careful with the wording choice. I never tell anyone
Poor choice of wording
I instead say this:
Better choice of wording
This makes the author think:
These simple word choices have helped me enormously.
I never underestimate junior devs. I have worked with some senior developers (over 10 years experience) and there code was worse than a junior co-op student. So just because they are senior or junior is not all that important; their work is really what speaks louder than years of experience.
Oftentimes, to encourage junior devs and get them participating in reviews, I will send them something to review for me: Something they can figure out, something they will find challenging but not totally beyond them. I may word it like below:
This helps me greatly again. This helps because it clearly shows that I am not the only one who reviews, but they also do reviews and they are also part of the process. It shows that the whole idea is to produce good, clean code and ask for help if needed. The review process is a culture so we really need to work at it.
Now some people may worry that if they do the above, the junior devs will lose respect because they just did something you could not do. But that is far from the truth: asking for help shows humility. Plus there are plenty of situations wherein you can shine. Finally if that is your fear, you have self-esteem issues. Lastly, maybe I really do not know it: I mean some of these devs have algorithms fresh in their head because they just studied them a month ago.
Anyways, back to juniors and reviews. You should see the look on their faces when they figure it out and send me a reply. I then may tell them, "OK, let me make the changes and if you are happy with it, please close the issue."
They feel awesome having the power to look at my work and say: "Yes, your changes are good. I closed the issue."
I never fix the code myself though because:
But I will suggest ideas and code snippets in my comments to help out the author. Please note that sometimes my reviews are simply asking the author that I do not understand their code; there may be nothing wrong with their code. They may need to change variable names, add comments etc. Thus, I will not even know what to change in that case; only they will.
If you keep doing reviews, you will, sooner or later, have a really good idea of the knowledge level of each developer on the team. Knowing this is really useful and you need to take advantage of it and unleash it. Here is how: If I review some code and see obvious areas for improvement and I know another developer may catch them too, I will get them to review it instead. Something like "Hey, I see a few areas that can be improved. Can you please review it in more detail and send your comments to the author?" This works great too because now I have 2 other devs working with each other.
If I am reviewing some work and I notice something that the whole team can benefit from, then I will create a hypothetical scenario and explain the issue in a meeting. I will start by explaining the scenario and ask everyone if they can find the issue or see an issue, get them involved. Get everyone to ask questions. Then finally present a better approach. If someone else has a better approach, I thank them and acknowledge in front of the team their approach is better. This shows I am not "my way or the highway" type of personality. Furthermore, I NEVER open someone's work and start pointing out the issues in a meeting, the author will not appreciate it--regardless of how nice and harmless I think I am being.
When I do reviews, I do not just look for good clean code but also what the code is doing. If the business requirement is: If an employee has been with the company for longer than 10 years, give them 5% increase. Otherwise, 2.5%. The first thing I check is if it is actually doing that. Then I check if it is doing it it in a clean, consistent and performant way.
If I do a review, I make sure to follow up or no-one will take the reviews seriously.
I used to work with someone years ago who would do a review and cc the dev manager, and the QA manager but both managers came from a business background or had little development experience. He only did this to impress them. No one liked it and that is when I told myself I would never make that mistake.
The other thing he used to do is pick on programming style and was convinced his style is the best ("My kungfu is way better than your monkey style..."). That was another lesson for me: there is always more than 1 way of solving a problem.
Answer to some of your numbered questions
No, please see reasons I stated above.
Yes, try to use sentences and a tone that is friendly but yet requires attention. Be as clear as you can. State what the issue with the code is, how to improve it. DO not simply ask to change it. But provide reasons.
Like I said, you can use the tool I use or another tool. Do not use email or word documents because they get lost and it is hard to keep track of it.
Mostly what I do is to check the delta (changes only). But you need to have the overall picture in mind to ensure nothing is broken and it follows the architecture.
Final Thoughts
I personally think the word "code review" is a poor choice and do not know how it got started. They could have chosen a much better and less authoritative word.