Improve code reviews in 10 steps

When collaborating daily, determining "good enough code" is challenging without consolidated practices. Beyond standard tools like Eslint or Prettier, assessing code standards in state management, architecture, design patterns, and performance is intricate. The absence of guidelines makes it difficult to address these nuances, compounded by developers' reluctance to admit mistakes. To tackle this, establish centralized best practices to streamline evaluations and foster a culture of openness and continuous improvement. Encourage an environment where developers feel secure admitting and rectifying errors, promoting overall team growth and the production of high-quality code.

Step 1: Understand Requirements

First and foremost, grasp the context behind the provided changes. Making modifications without a clear understanding of what the code is intended to achieve is a significant pitfall. This is why developers frequently incorporate links to Jira or other team collaboration platforms in their pull requests. Connecting the code to its purpose enhances comprehension and facilitates smoother collaboration within the team.

Step 2: Verify CI&CD Results

Reviewing code with broken unit tests or e2e tests is counterproductive. In such instances, the code is likely to undergo changes as developers address implementation issues or modify the broken tests. While there are exceptions, such as during prototyping, building MVPs, or POCs, the general rule is to avoid checking PRs that yield broken CI&CD results. This practice ensures that the codebase remains stable and reliable throughout the development process.

Step 3: Be Pragmatic

Assuming linting, tests, and build for a specific PR are all in order, and business requirements have been verified, it's time to embark on the review process. A pragmatic approach is key at this stage. Remember, Code Review is an integral part of your responsibilities, but excessive time spent on it may detract from actual coding and potential improvements.

Being overly rigid in adhering to rules can be deemed technological fanaticism. Consider the example below:

function sum(a, b) {
   return a + b;
}

If your project currently avoids function declarations, and the team prefers using function expressions, introducing a rule for consistency is a valid point in the PR. However, if there's a technical rationale, such as leveraging hoisting with function declarations, this argument becomes less valid. Strike a balance by focusing on meaningful points without becoming overly prescriptive.

Step 4: Focus On Real Problems

Examine the following React code:

import React from 'react';

const MyButton = ({ label, onClick }) => {
  const handleClick = () => {
    // Your custom logic goes here
    console.log('Button clicked!');
    // You can also call the provided onClick function if needed
    if (onClick) {
      onClick();
    }
  };

  return (
    <button onClick={handleClick}>
      {label}
    </button>
  );
};

export default MyButton;

Direct your attention to the handleClick function. Suppose someone suggests the following comment in the PR:

Tom Riddle says: Please use useCallback hook for better performance!

This appears to be an unsubstantiated comment about a non-existent problem. What specific performance improvement is the author referring to? Has it been measured? Is it necessary, or does it introduce the overhead of maintaining the dependency array in useCallback?

As emphasized earlier, focus on identifying actual problems such as potential defects and clear performance issues, like the absence of side effect cleanup.

const MemoryLeakExample = () => {
  const [data, setData] = useState(null);

  useEffect(() => {
    const fetchData = async () => {
      const response = await fetch('https://api.example.com/data');
      const result = await response.json();
      setData(result);
    };

    fetchData();
  }, []);

  return (
    // Add your component JSX here
  );
};

export default MemoryLeakExample;

This code snippet could be grounds to block the PR as memory leaks pose tangible risks to the application. Addressing real problems ensures the codebase remains robust and reliable.

Now Tom Riddle says better: You have a memory leak in useEffect.

Step 5: Focus On Good Enough Solution

While resembling the previous point, this step emphasizes a different aspect. What does good enough mean? It's a code version that is functional, devoid of bugs, passes green CI&CD checks, and introduces no performance issues or disrupts existing repository patterns without justification.

For instance, if someone proposes adding Redux to your app while you're already utilizing Zustand, and their primary rationale is their prior experience with Redux, this PR may not be suitable for acceptance. Prioritize solutions that align with the current technology stack, maintain functionality, and adhere to established patterns unless a compelling reason for change is provided.

Step 6: Block The PRs That Are Too Big

The developer's responsibility is to create valid and good enough code. If he needs a review, he needs to prepare the code for review in the best possible way. Meaningful commits, suggestions in the PRs, or descriptions of the changed scope may be helpful. If you don't have a correct base to review, spending time on it is just time almost wasted.

In addition, code review is a way of sharing knowledge between team members. Doing it should verify the code provided by others and give you a scope of changes.

Step 7: Calls May Be A Better Choice

When dealing with a complex scope, consider organizing a call with team members to discuss and explain the changes. This approach can be more efficient than engaging in ping pong communication through comments. While it's essential not to overuse this method, it can be a better choice in certain situations, especially based on experience, time constraints, and individual preferences. Keep in mind that opting for calls is a valid decision within the spectrum of communication choices during the review process.

Step 8: Write Down Project Rules

To prevent conflicts in each PR regarding project rules, collaborate on creating a document outlining the permissible rules for your projects. Describe coding patterns, architecture, and overall project goals. When someone suggests changing a pattern, direct them to the README.md file and encourage them to propose a PoC with their ideas. Often, developers may halt a discussion at this point if they're unsure about their suggestions or perhaps just had a challenging day φ(*￣0￣). Establishing clear project rules fosters a shared understanding and minimizes unnecessary debates during code reviews.

Step 9: Use Review Templates

Consistency is key in code reviews. If comments vary in style and importance, it can be confusing to developers. Introducing consistent review templates, especially using md syntax in platforms like Github, not only streamlines the feedback process but also enhances the overall visual appearance of PRs.

Here's an example of review templates:

**Must Be Changed**

This comment indicates a situation where a portion of the code poses a risk to the application.

**Suggestion**

Overall good work, but there's room for improvement – not mandatory, but encouraged.

**Question**

Simply seeking clarification. Why was a specific part added, and what is its purpose? This should be addressed as well.

...etc

Adopting such templates promotes clarity, accelerates the review process, and contributes to a more organized and effective collaboration within the project.

Step 10: Keep a Cool Head

Maintain a positive and supportive tone in your comments. Remember, we are all human, and everyone, including architects and seniors, makes mistakes. Avoid offensive remarks that highlight someone's lack of knowledge or skills. Such comments can be demotivating and create a negative atmosphere in the team.

Instead, focus on supporting your team members, offering constructive feedback, and collaborating to create the best possible project. A happy team leads to successful projects, satisfied developers, content clients, and the resources needed to continue working for years to come!

Real-Life Scenario: Lessons Learned

To illustrate the impact on code reviews, let me recount a situation from the early days of my career when I worked on an employee work time management app. The project involved typical CRUD operations—stats gathering, permissions, and other straightforward functionalities.

This was around six years ago, and terms like CI&CD were not yet standard; they were just beginning to find their way into projects. The tech lead in this project had a notably cocky demeanor, consistently projecting an "I know everything best" attitude. Collaborating with such individuals can be challenging, fostering an environment where other developers, including myself, felt undervalued.

The code crafted by the tech lead was always asserted as the best. When queried about certain parts, responses were dismissive, claiming it's a "best practice" or "you should do that and that." In retrospect, I've come to realize that such answers are devoid of technical substance; they are essentially baseless arguments. Programming is a scientific endeavor, and science does not rely on vague assertions like "you should." Instead, it addresses questions or attempts to do so methodically. If a direct answer is elusive, it breaks down complex problems into manageable components and addresses them individually.

Experience alone does not guarantee superiority. Having ten years of experience does not automatically make one a better developer than someone in a Junior role. In my view, Senior devs might be less technically skilled than Juniors, but they bring essential soft skills—management, communication, responsibility, experience, and the ability to translate business requirements into actionable tickets.

The key takeaway is this: Higher experience in a project doesn't necessarily equate to being a better developer. While it could be the case, it's not an absolute rule. Individual capabilities, regardless of experience levels, should be evaluated based on tangible skills and contributions.

Summary

These are fundamental principles and suggestions to enhance your code review process. They aren't golden rules but rather ideas that can be implemented, and you might discover even better ones tailored to your team's needs. The optimal choice is always the one aligning with your team's requirements and fostering a collaborative and efficient development environment.