Code reviews are amplifiers and there are different solutions

They amplify the good and the bad, and this course will show you how to maximize the good and eliminate the bad. Some of the major topics we will cover include establishing fundamental processes of code reviews, creating an excellent pull request as a reviewer, and providing effective feedback as a reviewer. By the end of this course, you will have the soft skills to participate in code reviews as a professional to ensure they provide maximum value. Before beginning this course, you should be familiar with the basics of software development with any technology stack in a professional setting, writing some code, committing, pushing, creating a pull request, and merging. I firmly believe that poor software design leads to software that is difficult to change in response to growing needs and encourages buggy software that saps the productivity of computer users everywhere. I'm always trying to find out what designs are practical, what approaches lead people into trouble, how we can organize our work to do better designs, and how to communicate what I've learned to more people. So much to do before even our first pull request and code review, but as they say, preparation is critical. The less you spend setting up the basic rules and processes, the more time and effort you will spend on each code review, accumulating quickly. Agree on and implement style guides, install and leverage static analyzers, and make it a habit, and you will be rewarded with work that is not clogged up with tedious checks and fixes. Having at least one reviewer who understands the business is paramount. I understand that some teams have their time and resources stretched; I've been there, but try to have a second person on every significant feature or component who at least knows the basics. Try to establish the right attitudes between yourselves. Senior developers aren't always right because a human being always right does not exist.

Accept that more than one good solution may exist to any problem. Finally, except for most critical bugs, PRs shouldn't hang for three days or a week for whatever reason. If it does, find the reason and try to remedy it. Up next, Submitting a Great Pull Request, how to ensure a smoother review experience. The practice of pre‑approving only minor issues speeds things up a bit, so that's great, but it should not be done frivolously. I have also seen pre‑approved pull requests that still require much work, and I can't agree with that practice. So please, be careful with this particular advice and use it sparingly, if at all. 

Build trust and relationships in your team. This is the primary preventive measure, and it is mighty. Chat to each other about life, discuss common interests and hobbies, and go for lunch together. Every lunch is a mini team‑building exercise. If you're working from home, have occasional short virtual meetings that are not work-related, at least a bit, even if you're an introvert. Why? Because we naturally behave better towards each other when we get to know each other better. So when you build trust, your code reviews get much faster. As a reviewer, you accept feedback better, and you don't imagine it as aggressive because you had a nice chat with the other person just yesterday. If you are a reviewer, you will naturally be more willing to review the code diligently and thoroughly, hopefully not too leniently. 

Remember, the later you catch a bug, the more expensive it is to fix. Production is too late, but ideally, you want to catch bugs before any official testing and even before the code is merged into the rest of the codebase. Another goal is to catch code quality issues. Almost no one outside the development team ever cares about code quality, not the paying customer or the end user. They just want the software to work. But it is practically a law that quality decreases as the software grows and increases in size and complexity. If developers don't want their progress to grind to a halt, maintaining code quality is in their best interest. And again, it's cheaper to do things right the first time and catch issues before merging rather than promise to yourself that you'll refactor it later. Code review is also a learning opportunity for the pull request creator. It's a chance to receive feedback that allows you to gain knowledge. How to write better, cleaner code. Maybe you didn't follow a good principle or pattern you've never heard of. Perhaps you reinvented the wheel, and someone pointed out that you could've used an existing function or library to achieve the same goal faster. Code review is also a learning opportunity for the reviewer. They might learn everything I mentioned from the person who created the pull request. As such, code review serves as an exchange of best practices and experiences. It is a great opportunity to grow as a programmer. Finally, developers may get out of sync when working on their tasks and not communicating with each other. Looking at each other's code is a great way to check if you're still on the same page. In a way, a code review is a virtual place where people can gather and discuss specific work‑related matters revolving around code. In a way, it can be as valuable or even more valuable than daily or weekly stand‑ups. In essence, code reviews should serve you and your team to write better software. It's meant to be a process that leads to a win‑win situation.

• Understand the process and general principles of refactoring
• Quickly apply useful refactorings to make a program easier to comprehend and change
• Recognize “bad smells” in code that signal opportunities to refactor
• Explore the refactorings, each with explanations, motivation, mechanics, and simple examples
• Build solid tests for your refactorings
• Recognize tradeoffs and obstacles to refactoring

Accept That There Are Different Solutions

It's best to remember that a problem almost always has more than one solution. Of course, not all of them will be equally good, that's for sure. But it's also very likely that more than one solution will be reasonably good. If, for example, you are a reviewer, you might have your favourite solution, but the author's solution may also be valid. Distinguish between standard best practices and your taste, and don't force your solution on the author if theirs is also acceptable.

What to look for?

1. The code is well-designed.
2. The functionality is suitable for the users of the code.
3. Any parallel programming is done safely.
4. The code isn’t more complex than it needs to be.
5. The developer isn’t implementing things they might need in the future but doesn’t know they need them now.
6. Code has appropriate unit tests, and tests are well-designed.
7. The developer used clear names for everything.
8. Comments are clear and valuable and mainly explain why instead of what.
9. Code is appropriately documented
10. The code conforms to our style guides*

Timely Feedback

1. Our goal: Optimize for the speed at which the team can produce a product together, as opposed to optimizing for the speed at which an individual developer can write code.
2. Provide timely feedback on coworkers' PRs to achieve more incredible dev velocity and reduce churn: one-shot review < 1 day from receiving
3. Decline a PR and explain your reasons: lack of time, not knowledgeable
4. Check for smaller PRs that can be reviewed and approved (with suggestions) quickly to promote dev velocity and reduce frivolous churn (from nits or merge conflicts) from staying open endlessly
5. Block out time as needed to go through larger PRs that need more time to review, understand, and evaluate
6. The first PR feedback round is often the most critical/crucial for shaping the final technical direction and product outcomes
7. Don't Let the PR Hang for Days: A code review should be done within hours. It shouldn't stay open for days. This is an important convention that everyone on the team needs to agree upon, and it's the responsibility of both parties. The rest of this course is dedicated to making code reviews as smooth as possible, with concrete tips for both reviewers and reviewers. If you follow this course's advice, hanging PRs will simply not exist.

Security: Compliance Obligations:

1. Always ask if not sure about our obligations
2. Mark compliant module reviews clearly in the PR Description
3. All module outputs should be encrypted
4. Check outputs for Personal or Tenant information exposure risk
5. All data written to StdOut and StdErr are system metadata…

Focused Changes:

1. Make PRs fast by keeping them focused
2. A minor PR has less surface area for churn. So it is easier: for the reviewer -- to timebox, to review entirely and confidently, and to trust with an early approval, for the author -- to revisit in parallel with other work, to merge without conflict, and to complete in a timely fashion.
3. A PR tends to go faster if: Changes are profound: require significant review in a single vertical area, Changes are broad: and require external review across a horizontal capability.
4. Not Focused: Mixing a PR to reduce the number of columns to optimize storage (horizontal) and a new offline metrics implementation (vertical) is suboptimal.

When Code Reviews Go Wrong

There is a dark side to code reviews; however, as a process, code review is not 100% force for good. Instead, it can be seen as an amplifier. Let me repeat that. Code reviews are an amplifier. They amplify the good and the bad. If the organisation's culture is healthy, code reviews bring some or all of the benefits I've discussed in the previous clip. It amplifies the good. However, poor company culture, difficult personalities, and, most importantly, unprofessional behaviour may get amplified through code reviews. In such cases, the cost of code reviews never pays off. You may get into unnecessary arguments that slow development, confrontations, and arrogant or condescending comments, all leading to decreased morale. The fear of attracting such negative comments may lead to developers hiding defects or cutting corners and, in the worst case, contributing to staff turnover. In other words, people leave the team or the company out of frustration. This course isn't about establishing a company‑wide corporate culture, and it's not exactly about dealing with challenging personalities, but specifically, the code review culture and the appropriate professional behaviour in its context. And it's crucial to improve code quality and foster a constructive exchange of ideas and knowledge.

Handling Difficult Reviewers and Reviewers

Talking face to face or in a call with a webcam resolves many disagreements, but not all of them. Difficult personalities do exist. However, you choose to define that. For example, a reviewer makes the same mistakes repeatedly and refuses to learn, forcing the reviewers to leave the same comments trying to correct the same issues. Or someone unwilling to implement suggestions at all. Or a reviewer who might be considered overly critical or pedantic blocks pull requests whenever there's a double space in a comment. Or they act as gatekeepers and always insist that their suggestion be implemented, which effectively makes it a command, not a suggestion, even if it is phrased nicely. Not only does this slow down the process, it can also decrease the team morale to the point where everyone wants to leave. In such cases, the highest possible code quality is not worth it. I hope that you will not find yourself in such situations, but if you do, then a good process is to try to discuss one on one to gain the deepest possible understanding of the other person and find common ground. If you can't agree, seek an outside opinion. Then, if you get two out of three, agree that X should be done, or three out of four are in favour of that other thing, then you go with the solution chosen by the majority. If all else fails, escalate to a senior and let them handle it. If it works out in your favour, great. If not, accept it and hope for a better outcome next time. If this happens repeatedly and you feel it's not worth staying, consider moving. If you can't change the environment, try to leave the environment.

Summary

I like the Refactoring book from Fowler: https://www.amazon.com/Refactoring-Improving-Existing-Addison-Wesley-Signature/dp/0134757599/ref=pd_lpo_1?pd_rd_i=0134757599&psc=1  Primarily because it gives names to the stuff, I already do. The main advantage of that is communication with other people:

• for junior programmers, the book gives them the steps on how to do things
• for senior programmers, the book allows me to talk with other senior programmers that read the book with fewer words.

You might have trust in your team, but strong disagreements happen sometimes. There's just no way to avoid them altogether. A typical indicator is three comments on the same thing. The initial comments by the reviewer, pushback, and then another pushback. The moment you notice this, consider taking things offline. If you are both in the office, a face‑to‑face talk is best. Go to a whiteboard, get a coffee in the company's kitchen, wherever you feel like it, and discuss it until you resolve it. If you're working from home, try chatting or a call with a webcam. If you work in a large or medium‑sized company, you must have observed a chain of emails that lasted for a week, and you thought, well, this could have been resolved in an hour if it were a direct conversation. Pull requests are no different. Whenever you agree, either the reviewer or the reviewer should then close the conversation on the pull request by saying, as per our discussion with X offline; we decided to go for a solution. 

References

• Style guides for Google-originated open-source project: styleguide | Style guides for Google-originated open-source projects
• Code Smells: Code Smells (codinghorror.com)
• Best Practices for exceptions - .NET | Microsoft Docs
• C# at Google Style Guide | styleguide
• Google C++ Style Guide