DEV Community: Reviewpad

How to accelerate your code reviews?

Fábio Freitas — Thu, 20 Jul 2023 16:00:00 +0000

The code review process can often be tedious and filled with bottlenecks, making it slow and time-consuming. However, there are tools like Promyze and Reviewpad that can greatly assist in improving the code review process and saving you and your team a significant amount of work. In this article provided by Promyze, you'll discover effective strategies on how to expedite your review process.

Code reviews through Pull/Merge Requests have become an essential process for the majority of tech companies.
A recent study in 2022 unveils that 84% of the surveyed companies use it.
Several years of practice allowed practitioners to take a step back on this methodology to identify pain points and how to overcome them.
Besides, code reviews should always be continuously evaluated internally to improve how it’s done.
In this post, we discuss how to add more flexibility to the Pull Request process and, thus, improve the delivery time.

The lead time for change is the amount of time it takes a commit to get into production, and is part of the 4-key metrics defined by the DevOps Research and Assessment (DORA).
Engineering teams seek to optimize this delay to deliver sooner value to their customers. But before bringing solutions, let’s first define the problem:

During code reviews, what can slow down the whole process, create bottlenecks, and grow the lead time for change?

Narrowing the context and purpose of code reviews

The most often quoted obstructions to code reviews are the lack of human resources and the workload of engineers that don’t allow enough time for code reviews. A common reaction consists in adding more human resources to address the issue. Not so fast!

First ask yourself:
Is each code review truly necessary? Can we affirm that all the discussions within reviews are relevant and should happen there?

Indeed, what we observe in practice is that code reviews often lack a clear context and purpose. If you’ve opened a merge/pull request, what do you expect from that? From our point of view, we propose two primary purposes for a Pull Request.

#1 Code approval

I need to ensure my code is okay before merging ⇒ I need your approval.

In this case, it may raise friction from both sides, as reviews come late in the process.
Authors can feel reviews as a constraint that slows down their delivery, and a frustration to get many comments, with which they won’t always agree.
Reviewers can also adopt defensive behavior. What if they think: “I wouldn’t have done [the code] this way.
Should I tell this? Should it be blocking for merging? My colleague waits for 2 days; how will my feedback be welcome?”
So you can see that frictions can emerge from code reviews, mainly because we have the pressure induced by the quest for a satisfying lead time for change.

#2 Get help to improve the code

I’d like to show you my code and get your feedback to improve it ⇒ I need your help.

Compared with item #1, we’re looking to improve our code and learn from a peer.
In this context, it’s worth considering whether this review is part of the delivery process.
We may be convinced this code could be merged first. That’s something commonly observed in trunk-based development.

The workflows and policies are different based on the context of the PR, so it makes sense to define it clearly.

Another problem we observe during code reviews is the long discussion threads about implementation choices, design, and best coding practices.
And there, discussions get emotional and passionate: this is precisely what we want to avoid during reviews.

Do we still need code reviews?

Aside from this discussion, you may ask whether doing code reviews still make sense in 2023, in an era where we can find plenty of “DevOps automation” tools on the market.

We consider that, yes, indeed, code reviews are still necessary.
Even though automatic tools have become increasingly powerful, they’re good at detecting known problems.
They can’t prove the absence of problems. Think about this Dijkstra quote:

“Program testing can be used to show the presence of bugs, but never to show their absence!”

So during a Pull Request, we seek for what tools can’t detect.

Now let’s go back to our main point: how to address the aforementioned issues?

Do code reviews only when it makes sense

The concept of Fluid Pull Request

To answer the question: “Do all Pull Requests require a code review?”, we can explore the model Ship Show Ask. In this approach*,* PR authors are considered free of trust to evaluate themselves if:

The code doesn’t require a code review since the changes are trivial (Ship)
They’d like to show the code changes to colleagues and get their feedback (Show); This mode can sound totally counternatural as the feedback can be provided after the merge. This is part of the paradigm shift.
The code requires a review (Ask)

So instead of adding more human resources to review the code, we reconsider the necessity for each PR to be approved;
This is how we can positively impact the lead time for change. This concept is also called Fluid Pull Request.

Next generation pull requests with Reviewpad

Reviewpad is a GitHub app designed to automate your code review process and which implements the Fluid Pull Request concept.
A single configuration file in your Git repository will contain all your workflows and policies for your Pull Requests.
Based on the context, you’ll define rules to enforce or relax your security net. With Reviewpad, you can define rules such as:

If a file in this folder is edited, one person from this group should approve the code.
If the PR includes a new method in the code with an annotation @Critical, add a label ‘security, and require a double-validation. Rules can decide whether PRs can be automatically merged or refused, define a validation workflow, and more. Rules can address syntactic patterns in the code changes.

Here is an example of workflow to automatically raise awareness when a critical change is introduced:

labels:
  critical:
    description: Modifications to critical changes
    color: "#294b75"

groups:
  - name: owners
    spec: '["lina", "caris"]'

workflows:
  - name: changes to critical code
    run:
      - if: $hasAnnotation("critical") || $hasFileName("runner.go")
        then:
          - $addLabel("critical")
          - $assignReviewer($group("owners"), 1)
          - '$info("@samir: you are being notified because critical code was modified")'

Discuss best practices and design outside code reviews

Knowledge sharing should be collective

f you request a review to improve your code or show it, this shouldn’t be done under pressure before merging the code.
Engineers should instead find dedicated time to discuss code design and best practices, avoiding friction.
If you’re having a 1:1 conversation and debating about a best practice, you hold a discussion that may impact your whole team.
Indeed, if you come up with a decision, shouldn’t everyone agree with that? We can assume that you seek code uniformity and practice alignment.

So maybe it’s time to consider running dedicated workshops on your best coding practices, as 1:1 conversations should not be the right place for that purpose.

Best practices sharing with Promyze

Promyze is a knowledge-sharing platform for developers and provides integrations with IDE and code reviews (GitHub, GitLab, Azure DevOps, Bitbucket, and Helix Swarm).
With the code review plugins, you can create a best coding practice from a comment, and the code is sent to Promyze along with your best practice proposal.
You thus avoid long 1:1 discussions during code reviews.

Then, Promyze helps your team regularly run dedicated workshops to review contributions from developers (made during code reviews or within IDE).
It’s a suitable time to share knowledge and make technical decisions together and ensure everyone is aligned with the appropriate gestures to apply in the code.
This is a continuous improvement process for your best coding practices and to grow developers’ skills.

Want to accelerate your code reviews?

If you feel your engineering team has bottlenecks in their code reviews, we hope you found in this post some insights to improve your process:

Consider the necessity to approve each code change;
Consider dedicated sessions to avoid never-ending discussions during reviews.

We also showed that Reviewpad and Promyze can be complementary to accelerate the code reviews while fostering knowledge sharing.

Tell us if you’ve improved your lead time for change in your context and how you made it; we’d be happy to learn!

The Evolution of Code Reviews: The Beginnings

Fábio Freitas — Wed, 12 Jul 2023 17:00:00 +0000

Code review is now a vital part of software development in the age of DevOps. Understand the evolution of code review from its early days to its present shape. We'll look at how code review has adjusted to the demands of development teams, from time-consuming manual reviews to efficient automated ones.

With the rise of DevOps, the code review process has become a substantial part of software development. The right code review process ensures that our applications are highly available and can be built with high velocity.

Over the years, code review has evolved from manual processes to automated tools, adapting to the changing needs of development teams. As application development continues to evolve, it's essential for code review to keep pace and provide teams with the best possible workflow for ensuring code quality.

In this 2 part series, we will explore the evolution of code review from its early days to its current state. We'll look at how code review has adapted to the needs of development teams, the trends driving its evolution, and the future of code review.

In this first part, we will go through the pages of history to understand how the code review process came into existence, and how some of the shortcomings led to major disruptions.

Whether you're a team leader trying to set the right practices for your development team, just a seasoned developer or simply starting in the software development world, this series will provide valuable insights into the importance of code review. Join us on a journey through the evolution of code review, and discover how it continues to shape the future of application development.

The Dark Ages

In the early days of software development, code review was a manual process. Developers would physically sit down together and review the code line by line, discussing any issues or suggestions. While this method was effective, it was also time-consuming and not scalable for large development teams.

The introduction of Git in 2005 revolutionized how software development teams worked. It scaled the way teams, and contributors could now collaborate as developers could submit their code changes as pull requests. These pull requests would then be discussed and reviewed by other team members and, once accepted, were automatically merged into the main codebase. This was the rise of version control and all its benefits, such as keeping a history of changes and being able to roll back to previous versions.

However, this method relied on manual reviews, and the feedback process was often slow and inconsistent. With Git, teams had the flexibility to conduct code reviews in any way they saw fit, which led to inconsistencies in how code was reviewed. Different teams used different processes and tools, and the process of manually checking the pull request still made it difficult to standardize the entire code review process.

Overall the code review process was very manual and slow. Cutting corners to get functionality out to production often meant disruptions and bugs.

When Disaster Hit

This is exactly what happened to the London Stock Exchange on February 8th, 2008. It's usually not the best time for a financial institution facilitating trade in Europe's financial hub to be down in the middle of a global recession. This disruption was caused due to a bug in the new trading system that was released then.

The impact was that trading came to a halt for almost an entire day, and till today the financial amount lost due to the impact is not publicly available. It is safe to say, though, there are significant losses for the Exchange and its customers. Given the scale of the incident and the impact, it had on the financial markets.

Another such example is that of Knight Capital Group's trading systems which experienced an incident that led to a significant financial loss of $440 million in just 45 minutes. The issue was due to a code change that was not properly reviewed and tested, underscoring the significance of a comprehensive and efficient code review process.

Conclusion

The code review process has become an integral part of software development with the rise of DevOps. Over the years, code review has evolved from manual processes to incorporating automation. This was due to the need to push changes faster while maintaining the stability of your systems.

However, meeting this requirement was something that was just not feasible with the way code reviews were being performed. Stress the system too much and you could have experienced great outages and disruptions such as that of the London Stock Exchange or…

By tracking the evolution of code reviews and being aware of the past issues, we know it is important for us to refine our coe review process. How we can do so is answered in part 2 of this series where we continue our evolution into the future!

The Evolution of Code Reviews: A Better Tomorrow

Fábio Freitas — Wed, 12 Jul 2023 17:00:00 +0000

In this second part of the evolution of code review, we explore the impact of emerging trends in application development. Discover the significance of the shift-left approach, the challenges posed by microservice architectures, and the rise of DevOps-related concepts like GitOps. Embrace change management and explore tools like ReviewPad that automate and streamline the code review process, ensuring swift and confident deployments for your applications.

This is the 2nd part of a 2-part series on the evolution of code reviews. If you haven’t already, check out Part 1 here. In the first part of this series we saw how code reviews were performed in the early days of application development, and how the process if stressed could result in major issues and disruptions. The story of the London Stock Exchange and … is just a couple of examples from a long list of stories where companies suffered because of their outdated or faulty code review process.\
In this second part, we will go over some of the trends that have come up in the world of application development. By understanding these trends we will be able to understand the needs for the future of the code review process. So lets continue on our journey.

Emerging Trends and Their Impact on Code Reviews

Over the past decade, with the DevOps revolution kicking off, we have seen many emerging trends that have redefined how applications are developed and how code goes through development to review to production. Many of these trends, coming, have had an impact on the overall code review process, and it's necessary to understand these trends to understand what lies next.

The Whole Org Shifts-left

Shift-left advocates for quality control measures of code to occur sooner in the development process, near the time code is being written. This approach enables teams to quickly spot and address any issues, reducing the possibility of defects and enhancing the general standard of the software.
As a result, code reviews became essential to software release as the QA testing phase merged into the code review phase. No longer was it enough just to ensure that the code was compliant and met set standards but also that it worked and did not result in any regression.

The Rise of Microservice Architectures

Microservices architecture became popular due to its ability to allow for more flexible, scalable, and agile software development compared to monolithic architecture.

However, with the microservice paradigm, it does become difficult to track how different services are related to one another, especially as your application scales. As a result, a change in one service could affect and even disrupt another.

So it became necessary that these dependencies are realized during the code review process. Some of the related services to the service experiencing the change may be extremely sensitive to disruptions and downtime as per the business goals set. Putting more significance on an effective and clean code review process.

DevOps, GitOps, NoOps…Why?

With the rise of the DevOps concept, concepts under its umbrella also emerged. These included concepts such as GitOps, AIOps, and even NoOps. Whatever type of “Ops” you may adopt, the goal is the same. Increase the velocity of product development while maintaining the availability and stability of the application.

Overall, what is expected is the use of automation wherever possible, combined with the shift-left mentality, to aid in the two goals mentioned. We have seen now how the significance of code reviews increased due to other emerging trends. With the “Ops” trend, we tried to lessen the pressure on the code review stage using automation to ensure that changes could roll out quickly but also safely with high confidence that it's not going to break anything.

The Future of Code Review

Considering the various trends in the application development space, one thing that becomes clear is that pushing changes with a high degree of confidence is key. This is where the concept of change management arises.

Change management is identifying, planning, and implementing changes in an organized and systematic way. It helps organizations deal effectively with change, minimize disruption, and maximize the benefits of change initiatives. Change management includes various activities, such as identifying the need for change, assessing the impact of change, communicating change to stakeholders, implementing change, and monitoring the results.

By realizing the need for change management, it becomes clear why new paradigms, such as Fluid Pull Requests, are gaining momentum when it comes to adoption.

Fluid Pull Requests are an advanced form of the Pull Request method utilized in software development. In the traditional Pull Request system, a developer submits code modifications for review and evaluation by the repository managers. Fluid Pull Requests attempt to resolve these challenges by modifying the code review process based on the risk of the change.

Risk could depend on multiple factors such as how the service being affected is related to other services. The type of change of whether it is a config change or a change in the front, and the number of incidents reported by the team performing the change.

Depending on the outcome of this assessment, various code review procedures can be established and automated to either lessen or increase the rigor of the code review process. For instance, if an important security patch is added, it might be necessary to include the security team in the code review process.

This is exactly the methodology that a team performing change management needs. Any tooling employed could instill confidence in accepting the change, and even automatically merge the pull requests when the risk score is lower below a certain threshold. Increasing both velocity and stability of the application development life cycle.

This is why we are seeing increased adoption of tools such as ReviewPad that automate the review process and instill confidence in code changes. Reducing the need for manual intervention and overall reducing the time it takes for customers to see value in the applications you are developing.

Conclusion

The rise of various trends such as DevOps-related concepts, developing in the cloud, and new architecture paradigms such as microservices greatly increased the importance of code review, making it a possible bottleneck to the entire application lifecycle. As a result further is tools such as ReviewPad, which enable you to perform change management with the speed and confidence required.

How Reviewpad is Putting the Reviewer Assignment Problem to Rest

Reviewpad — Fri, 24 Feb 2023 10:52:20 +0000

Streamline your code reviews with Reviewpad. Our advanced algorithm suggests the best reviewers for your pull requests, making your workflow faster and more efficient.

Are you tired of manually assigning reviewers to pull requests on GitHub? Do you find it time-consuming and difficult to ensure that each pull request is assigned to the most appropriate reviewer? If so, Reviewpad has the solution for you. With its new Code Reviewer Assignment feature, Reviewpad can help you streamline your code review process and ensure that each pull request is assigned to the right reviewer.

The Problem with Manual PR Reviewer Assignments

For teams working on GitHub, the manual assignment of reviewers to pull requests can be a time-consuming and error-prone process. As development teams grow in size, the need for an automated review assignment mechanism becomes increasingly critical. Without such a mechanism, developers must spend valuable time manually selecting the most relevant and available reviewers for each pull request. This process can take critical time away from development tasks, slow down the review process, and lead to bias in the selection of reviewers.

In addition, it can be challenging to ensure that reviewers have the necessary expertise to review the code in the pull request. This can result in longer review cycles, errors in the code, and delays in the development process.

Reviewpad's Innovative Solution for Effortless Reviews

Reviewpad's Code Author Reviewer Assignment Mechanism aims to solve these problems by automating the assignment of reviewers to pull requests. Reviewpad analyzes the past contribution history of code authors and uses that data to suggest the most appropriate reviewers for each pull request.

In this process, Reviewpad considers two factors when assigning a reviewer. Those are

Relevancy and
Availability

of the reviewers.

The relevancy evaluation is based on the authorship of the code. It analyzes how many lines of code have been written by possible reviewers in the latest version of the impacted source files.

The availability evaluation is based on the number of open reviews per reviewer, which ensures that reviewers with too many open reviews are considered unavailable.

If there are no reviewers available or identified by the algorithm, a random user within the organization will be assigned as the reviewer for the PR. This fallback mechanism ensures that the pull request does not remain unreviewed and that the code changes are still scrutinized by someone in the organization.

The Crucial Role of assignCodeAuthorReviewers Function

Reviewpad's Code Author Reviewer Assignment Mechanism is powered by the assignCodeAuthorReviewers function, a built-in function that takes several parameters to determine the most relevant and available reviewers for each pull request.

This function is critical as it allows teams to customize the review assignment process to fit their specific needs.

Understanding the Parameters of assignCodeAuthorReviewers

The assignCodeAuthorReviewers() function takes three parameters.

Total (Optional): This parameter specifies the total number of reviewers to assign to a pull request. The default value is one, but you can change it to assign more than one reviewer to a pull request.
Excluded Reviewers (Optional): This parameter is a list of reviewers to exclude from being assigned to a pull request. You can use this parameter to exclude specific reviewers who might not be appropriate for the review.
Max Reviews (Optional): This parameter specifies the maximum number of open reviews per reviewer. If a reviewer has more open reviews than this value, they will be considered unavailable and will not be assigned to the pull request. If we don't specify a value to Max Reviews, the algorithm ignores that parameter and considers reviewers regardless of the number of open reviews they have.

Examples of assignCodeAuthorReviewers at Work

Let's look at an example of how you can use the assignCodeAuthorReviewers function in Reviewpad.

Example 1

$assignCodeAuthorReviewers()

This example uses the default parameters for the assignCodeAuthorReviewers function. It will assign one reviewer to the pull request based on code authorship and availability.

Example 2

$assignCodeAuthorReviewers(3)

This example assigns three reviewers to the pull request based on code authorship and availability.

Example 3

$assignCodeAuthorReviewers(2, ["user1", "user2"])

This example assigns two reviewers to the pull request, but it excludes "user1" and "user2" from being assigned as reviewers.

Example 4

$assignCodeAuthorReviewers(1, [], 2)

This example assigns one reviewer to the pull request, but it limits the number of open reviews per reviewer to two.

A Step-by-Step Guide to Using Reviewpad's Code Author Reviewer Assignment Feature

Now that you have a better understanding of the assignCodeAuthorReviewers function, let's take a look at the steps of the workflow of the Reviewpad's Code Reviewer Assignment Mechanism.

Step 1: Sign up for Reviewpad and integrate it with your GitHub account. (Installation Guide ↗︎)

Step 2: In your repository, create a new workflow with the following code:

workflows:
  - name: Code Author Reviewer Assignment
    if:
      - '!$isDraft()'
    then:
      - $assignCodeAuthorReviewers()

Step 3: Customize the assignCodeAuthorReviewers function to fit your specific needs. You can use the parameters and variables mentioned earlier in this article to customize the review assignment process.

Step 4: Create a draft pull request in your repository.

Step 5: When the pull request is ready for review, move it to the Ready for Review status.

Step 6: Reviewpad's Code Author Reviewer Assignment Mechanism will analyze the relevancy and the availability of the reviewers and suggest the most appropriate reviewers for the pull request.

Discovering the Advanced Functionality of Reviewpad

It's important to note that Reviewpad's Code Author Reviewer Assignment Mechanism is a powerful tool that can help you streamline your code review process. However, it's not a replacement for good code review practices. It's still important to ensure that reviewers have the necessary expertise to review the code in the pull request and to maintain a clear and organized review process.

In addition, Reviewpad provides other powerful features such as automated pull request labeling, automatic pull request merge, and enforcing branch conventions. By using Reviewpad's Code Reviewer Assignment feature in combination with these other features, you can create a robust and efficient code review process that saves time and reduces errors.

Fluid Pull Requests

Reviewpad — Thu, 05 Jan 2023 17:32:00 +0000

By Freddy Mallet

Pull Requests have become the backbone of the DevOps movement: the faster, the safer, the better. But the code review step, this systematic and synchronized communication task between humans, inherited from ancient times, slows down the overall development lifecycle for an illusion of safety. It’s time to introduce the Fluid Pull Requests paradigm.

Back in 2008, GitHub introduced the concept of Pull Request and, by doing so, significantly eased the contribution to open-source projects. For the first time, it had never been so simple for two strangers to collaborate on the same code base. At that time, the only purpose of a Pull Request was to provide a safe way to suggest code changes and discuss them in a collaborative interface.

More than a decade later, 170 Million [1] Pull Requests are created each year on private projects and 30 Million on public projects. So the concept has been embraced by the overall software industry and has become a de facto standard. During the same period, the DevOps movement took off, and nowadays, Pull Requests are the backbone of any automated SDLC (Software Delivery LifeCycle). On each Pull Request, many highly valuable tasks are fully automated: building, testing, static analysis to spot bugs, deployment on staging environments, etc. All this DevOps movement could be summarized in one mantra: “The faster, the safer, the better”. Starting in 2015, the DORA (DevOps Research and Assessment) [2] team has identified four key metrics that indicate the performance of a software development team. Three of those four metrics illustrate this mantra: Deployment Frequency, Lead Time for Changes, and Time to Restore Service.

So today, the lifecycle of a Pull Request should be a matter of a few hours, but it is always a few days or even a few weeks. Developers managed to automate many things, but we still have in the middle of each PR lifecycle a systematic and synchronized communication task between humans: the code review step. This inflexible code review step, inherited from 2008, slows down everything and is the main bottleneck.

Why do we keep on accepting to pay such a price? Why do we keep on pushing the stop-the-line button on each PR? Because no software, no static analyzer, nor AI will ever be able to formally prove that a code change is safe and maintainable. Our tooling will keep evolving to spot more patterns but proving the absence of problems is an unsolvable challenge. So we rely on humans to do so. But humans are even less efficient at spotting problems, tracking a flow of execution, or keeping more than 10 variables in our working memory. This is especially true when each PR has to be reviewed. Developers are progressively desensitized, and LGTM symptoms arise. We’re just accepting to pay the price for an illusion of safety.

So what do we suggest doing? As we said, Pull Requests are nowadays the backbone of the DevOps infrastructure, so we can’t work without them. Moreover, today there is no code analyzer to prove the absence of bugs or vulnerabilities, or critical design flaws, so we still need developers to review some code changes. The only problem is that this code review is done systematically, regardless of the context, and always involves synchronous communication between humans. Here is where the Fluid Pull Requests paradigm knocks on the door.

At the core of this Fluid Pull Requests paradigm is the capacity to adjust the code review process based on the sensitivity of the code change, either to lighten or strengthen it. This sensitivity evaluation must take into account several different elements. We can mention, for instance, the seniority of the developer on a code base, the complexity of the code change, or the history of changes. Based on this sensitivity evaluation, you can specify and automate several code review processes to automatically approve and merge, do an asynchronous review, assign several reviewers, and spot the most sensitive parts of the code changes. For instance, if a database migration script is added, it could be relevant to involve the database team, but only to approve this piece of code.

I guess you got it: Fluid Pull Requests is an extension of the Pull Request concept, to increase both the throughput and safety of delivery. We still rely on human review, but only when it’s the most relevant, and in a very driven way. Instead of reviewing everything every time, we only have the most critical changes reviewed by the most impactful reviewers.

References

[1] Octoverse Report 2021: https://octoverse.github.com/writing-code-faster

[2] DevOps Research and Assessment: https://www.devops-research.com/research.html

Why Developers Should Care About Knowledge Sharing

Adriano Martins — Mon, 18 Oct 2021 09:18:44 +0000

Sharing knowledge usually comes as the last step of a process, or even worse, as something optional, that can be passed on if need be. This makes it easy to skip on this step if you feel you’re tired, or your team is busy. Developers are almost always creating new technologies and figuring out new solutions. Odds are, whatever knowledge you acquire won’t be available anywhere else. The time has come to remind everyone of the fundamental benefits of making whatever knowledge you’ve acquired available to your colleagues.

Why it helps you

Knowledge sharing is the second biggest problem software companies face. It affects everyone, so it also affects anyone. With the advent of remote work, this issue may become even more important. First of all, let’s appeal to our own self-interest. Sharing knowledge doesn’t only help our team, it actually helps those who are imparting the knowledge in more ways than one.

Let’s see:

You are advancing your own knowledge. By explaining what you’ve learnt to someone else, you are consolidating what you’ve learnt. You will retain it better, systematise it better, and, frankly, you’ll probably advance it by seeing it through others’ eyes. You will have to consider what you’re sharing and think twice about it. That reflection will bring with it valuable learning.
You will be receiving feedback. No developer is an island. We learn from others. By sharing what you’ve learnt, you will be receiving other people’s thoughts on the subject, which will surely advance your own understanding. Maybe they will even correct some aspect of it that you misunderstood.
You will help future you. If you learn something and just file it away in the caverns of a closed pull request, odds are you may not remember it fully whenever you bump into the same problem again. If you write it down, though, and teach it to others, it will be right there for you when you need it. It’s the easiest way to only solve problems once.
You will be advancing your career. Sharing knowledge and contributing to the team’s success does not go unnoticed. If when people are in trouble and seek solutions your name pops up all the time, everyone will take notice. This is how promotions happen.

Why it helps your team

But sharing isn’t just about oneself, now is it? There must be advantages for the whole team. Let’s see what they are.

It helps build team spirit. This may sound like a meaningless phrase, but it isn’t. What goes around comes around, and a team that is accustomed to helping each other will resort to helping each other naturally. Giving everyone a hand shouldn’t be contingent on someone being in trouble.
You will be saving everyone A LOT of time. How long did it take you to solve the problem? An hour? Two? A whole day? Whenever someone on your team runs into the exact same issue, they will (depending on skill level or just sheer instinct) take roughly the same amount of time. If there’s an explanation available, though, that everyone has knowledge of… Not so much.
You will be building a knowledge base. It may seem like just a small addition, but small additions build up. If your team develops a culture of sharing knowledge, they will inevitably create a repository that will become a cornerstone of success.

How to do it

So, are there any best practices, aside from simply gathering the team and telling them, or writing stuff down somewhere public, such as an internal Wiki?

We do have some ideas:

If you’re using Reviewpad, which you should, the Comments from the Past feature makes it so whenever someone touches code with comments, they’ll be able to see what was shared when that code was made. Forget about re-working the same problems, or going down avenues that somebody else already found out that don’t work.
Whenever a team member explains something to you, encourage them to document it. If it was useful to you, it will likely be useful to others.
Much in the same way, whenever someone asks you for help, document it.
Whenever you are the one who needs an explanation, avoid asking in private. Ask on a public forum, so everyone can read the discussion and take advantage of it.
Make it easily accessible. Your knowledge base needs to be freely and easily accessible to everyone, at all times. Developers don’t particularly like over complicated wikis, so it’s up to you to create this space.

One of the reasons why we are building Reviewpad is making knowledge sharing easier and more convenient to everyone involved.

Reviewpad is about communication as much as it is about reviews.

Are you ready to give it a go?

Ship / Show / Ask With Reviewpad

Marcelo Sousa — Fri, 08 Oct 2021 14:36:42 +0000

Ship / Show / Ask is a methodology to get faster releases with pull requests. With Reviewpad you get a code review platform that allows teams to naturally apply this methodology and extend it to new scenarios that give you greater velocity and team communication by going beyond pull requests.

Pull requests have been widely adopted by software teams as a way to collaborate in codebases. For all their benefits when contributing to open source projects, when it comes to projects where individual contributors have the power to merge their changes, pull requests slow software development.

Pull Requests Are Slowing You Down

The first reason pull requests are slowing you down is that theyare being used to document developer work.

If you need to open a pull request for every change, regardless of whether it is a new feature or a cosmetic change in a file, inevitably pull requests will be a bottleneck to integrate changes into a product.

When you associate “pull request” to “unit of dev work” and combine it with automated CI/CD pipelines, you get an overhead of time + resources that doesn’t always make sense in a fast-paced environment.

The second reason why pull requests are slowing you down is that we have associated code reviews with pull requests to a degree that we now use them interchangeably.

The truth is that not all pull requests need code reviews.

By enforcing processes such as “every pull request must be approved by a team member”, which is not aware of the actual changes, we have created a wait time in the process that is unreasonable in many situations.

This coupling of code reviews with pull requests has another major pitfall.

Because reviewing code is hard and the tooling around code reviews in pull requests is quite limited, it is standard to constraint the number of changes in a pull request. This means that changes that should be pushed together have to be broken up to accommodate this process.

Ship / Show / Ask

Ship / Show / Ask is a methodology to get faster releases by categorising changes into Ship (merge without pull request), Show (merge with pull request + later review) and Ask (standard pull request with review approval).

This methodology is powerful but still relies on the existence of pull requests. This limits you to always having to discuss changes between branches and trigger some automated process even when you just want to ask. It can also happen that you might need to do a later review but the changes were wrongly classified as Ship.

We think there’s a better way.

Code Reviews without Pull Requests

Reviewpad decouples code reviews from pull requests so that you fully embrace the idea that not all pull requests need code reviews and that code reviews can be done without pull requests.

Let’s take a look!

Reviewpad – Creating a Review Between Commits

With Reviewpad it is possible to practice the Ship / Show / Ask methodology in a more flexible way. In fact, what we have been practicing at Reviewpad is a Ship / Show / Ask / Revise methodology.

Ship

If you want to push a change to the main branch and you have the confidence to do so, you should just do it.

You shouldn’t have to wait for the CI checks or a review approval on a pull request to do so. Anyway, you typically run some CI pipeline in the main branch so you can always backtrack if something goes terribly wrong. You should have the power and trust to simply push changes.

Show

A Show process is when you run the CI checks over a branch before the merge, merge and only later get a review.

Although you can do this with a pull request, with Reviewpad there is really no need to do so. You can simply wait for the CI pipeline to finish when you push to a feature branch, merge, and then open a code review between those commits in the main branch. You can choose to open this code review to get later feedback between the branches or in the main branch itself.

Ask

The Ask process happens when you want feedback on our ongoing work.

Here we pause. We make our changes on a branch, we open a Pull Request, and we wait for feedback before merging. Maybe we’re not sure we’ve taken the right approach.

Unfortunately, for many developers, the idea of a pull request is more of a “work done” than a “work in progress”. In fact, mixing the two can be quite confusing because sometimes you just want early feedback on code that you know will not pass the CI checks.

Reviewpad allows you to create a code review between commits to get that feedback, revise it, close this code review and then open a pull request kicking off the official merge process in a team. That encourages more collaboration and communication about the code.

You can use the same code review interface regardless if you what you are reviewing is a pull request or just a diff between two commits.

Revise

So what happens if you want to revise some code that went into the main branch? This could be useful in the case of a huge load of pull requests with no time to review them or because you wrongly considered changes to be in “Ship” mode.

By going beyond the limitation of reviewing code with pull requests you can revise code in a single branch.

This allows you to create code reviews that document specific parts of the code, or to keep track of changes when you spend some time away from the project.

If you find issues in this mode, you can always connect to the code host by creating issues from comments.

Reviewpad – Creating an issue from a comment

Conclusion

With Reviewpad, you get a tool that provides the flexibility of new methodologies such as Ship / Show / Ask and beyond so that you can get the full benefits of code reviews and continuous integration without the limitations of pull requests.

You can try now at beta.reviewpad.com.

Managing expectations in code reviews

Reviewpad — Mon, 06 Sep 2021 10:55:57 +0000

Best practices are one thing that surely matters. Improving standards is always good. Who could argue against that? But what do you expect to get from code reviews? How do you expect the experience to go? Managing these expectations, especially when it's often so hard to measure results, can be daunting. Here are some thoughts on how to manage this.

Embrace the qualitative

Code reviews are about continuous improvement. That means it won't always be easy (sometimes it will be impossible) to quantify how much you've improved. Any metric you can come up with to measure return on investment (ROI) is likely flawed, and will probably tell you as much about the quality of your other processes as the reviews themselves.

We do know some things, such as the skyrocketing costs of catching bugs later, but the value of code reviews far exceeds that of simple debugging. The kinds of improvements code reviews achieve can sometimes only be felt years later, when an entirely different team is charged with updating the codebase and finding easily readable code.

Some things you just can't measure. We need to accept this.

Expect people will be reading you

This goes for several stages of the process. Sometimes developers will tend to forget that all their notes and comments will be read by a person that is also a teammate. A person with feelings, issues, and varying degrees of seniority and skill. Sometimes developers will tend to forget that their code will have to be read and understood by people over time.

This means you must be mindful of what you're telling others, but also what you're asking of them. A couple of examples:

Massive pull requests will probably lead to shorter and more superficial comments. Is this what you want?
Reviewers will need to place themselves. Have you provided the context that will allow them to fully understand what you're going for?

Always keep in mind: if you're typing it, expect someone will read it.

People make mistakes

One of the hardest things to manage regarding code reviews is the frustration that comes from reviewing bad code. Lots of mistakes, bad choices in terms of style, and unreadable code.

This, however, is almost inevitable. You're going to have to deal with bad code sometimes.

It's important to not let that frustration boil over into aggressive or hurtful comments. Communication must always be geared towards improvement, both of the codebase and the team. Code reviews are a great way for everyone to learn from mistakes, but that depends on the kindness of the reviewers.

Be mindful of people's schedules

Full-time code reviewers are rare. And even if they weren't, they would have a schedule anyway. Sometimes we expect our colleagues to pick up whatever is our priority as if it is also theirs, but that's not how it works.

Code reviews are often treated by teams like an afterthought, something that can be done by anyone "as soon as they have a minute". This is never the case, and reviewing is a task that should be treated with the same consideration as any other. Assign your reviews in due time, and expect people will be busy.

Best practices matter

Your expectations of code reviews should always take into account how they are being done. Are you posting the pull requests to a public channel and expecting anyone to pick them up? Then you shouldn't be surprised no one does, or if it takes them a long time to do so. Are you bundling up all the reviews and doing them at the end of the week? Then you should expect a lot of confusion. Are you reviewing for hours on end? Expect mistakes.

The full benefit of code reviews heavily depends on them being integrated into a continuous code-review-centric operational model, and the more your practices reflect this, the more you can expect from them.

Code reviews are about your team

An underappreciated aspect of code reviews is how they are a collective endeavor. We believe in them as a form of fomenting co-ownership of the codebase, and always tell reviewers to not think of what they're looking over as someone else's work. It belongs to the whole team.

Code reviews, when done right, will expand the whole team's knowledge of the product, and help everyone learn with both the mistakes and the skills of everyone else.

This is probably the most important thing you can expect from code reviews, if you make them work in that way: you can expect them to be not only a guarantor of your code's quality, but also a true engine of team building.

What are your expectations for code reviews? Did we miss something important? Drop us a line on Twitter (@codereviewpad) and let us know!

Mitigating Conflicts In Pull Requests

Marcelo Sousa — Wed, 01 Sep 2021 16:13:13 +0000

One of the most annoying tasks for a developer is to fix git conflicts. You open a pull request, ace the code review process, all checks are passing, and when you go for that merge button you find out that it is blocked because of git conflicts. Wouldn’t it be nice if you could avoid this situation?

Developers that use a git flow branching model rely on 3-way merge tools (like git-merge or kdiff3) to automatically merge their changes.

Unfortunately, these tools are unable to resolve all concurrent changes made to files in the branches involved in the merge. As developers, we experience these unresolved concurrent changes as git merge conflicts. As the name suggests, the existence of a git merge conflict prevents the merge action.

The exact definition of a git merge conflict depends on the algorithm used to compute the merge commit. The most widely used algorithms are textual in nature – a conflict in a file means that the branches involved in the merge have both made modifications to the same line in a file. Nowadays, there are much more sophisticated algorithms to compute merge commits than what is offered in GitHub, Gitlab, or Bitbucket. For an overview check out the article Verified Three-Way Program Merge – we do plan to incorporate some of them soon into Reviewpad.

However, a merge is and always will be a sensitive operation. That is, some concurrent changes really do conflict and require human intervention.

The reality is that git merge conflicts are relatively common in highly collaborative teams.

There are currently 27 pull requests blocked in the google/guava project because of git conflicts.

While some conflicts are unavoidable, it is possible to create awareness of their future existence so that you can mitigate their effects.

Reviewpad’s Concurrent Pull Request Analysis

At Reviewpad, we believe that git conflicts are being discovered at a very late stage in the review process. That is very annoying because sometimes after you have gone through the entire review process you find out that a recently merged PR just introduced git conflicts. Now, you need to restart the entire process.

So, to reduce this effort we have built into Reviewpad a concurrent pull request analysis that provides information about future git conflicts.

Information about conflicting changes with ongoing reviews at google / guava – #5654 Remove redundant bit masking.

The way Reviewpad keeps track of potential future git conflicts is by checking if modified files in a particular pull request are also modified in other open pull requests that share the base branch.

We have been using this check in two use cases that improve the review process.

Improve reviewer selection

Finding the right reviewer can be difficult at times in highly collaborative projects. Who better to review than a developer who is also changing the same parts of the code, or a reviewer of such PR?

Example of pull request without reviewers that has conflicts with concurrent PRs.

In the video, we can see that there is another open PR that is also changing the ci.yml. The author of that pull request could be a great candidate to review this pull request. Ideally, both developers could collaborate so that all conflicts could be avoided once one of the PRs is merged.

Improve merge strategy

As we mentioned in the beginning of this article, there is nothing more annoying than a pull request that is ready to merge but will be blocked by the merge of another PR. Although we can’t completely avoid this situation, we have integrated this check into Reviewpad’s Release Board so that it becomes evident.

In this first version, we have decided to restrict this information to pull requests that are ready to merge. Here’s what a PR card with conflicts in concurrent PRs looks like:

Pull request card that is ready to merge and that has conflicts with another PR that is also ready to merge.

In the following video, we go over the simple flow of approving a PR, seeing the conflicts in the Ready to Merge column and also the effect of merging this PR which moves the conflicting PR back to the Git conflicts column.

Integration of concurrent PR check with Review Board in the Ready to Merge column.

Cool. How do I check this out by myself?

We have a public beta version of Reviewpad available at reviewpad.com/get-started. You will need to create a new account and once you log in for the first time, you will see the following page to connect to a code host:

Connect to code host page on Reviewpad.

You can connect to GitHub through our OAuth app or manually add a personal access token. The OAuth requires minimal scopes to be able to read and comment on public repositories.

And voilà – you are ready to get started with Reviewpad!

We are just getting started with incorporating features that improve the merge experience in pull requests. Feel free to reach us on our community Slack with requests!

Pull Requests for Dummies

Adriano Martins — Mon, 30 Aug 2021 09:48:50 +0000

Debate about code reviews is often dominated by debate about pull requests. The confusion between the two is so common we have made it a point to clarify that they are not the same quite often. So what are pull requests? And why are they called that way?

Imagine you're writing a book.

(You knew that was coming.)

In this case, you're writing collaboratively. Let's say it's a non-fiction book, or a technical manual. Something that requires a certain degree of responsibility and accuracy.

You will need to figure out a strategy to make sure there are as few mistakes as possible.

How about… version control?

If this is a collaborative work, then it stands to reason that you will want to keep your file (let's say a Word document) somewhere everyone can easily find it and work on it. Let's say you keep your manuscript on Dropbox.

You (or any one of the other authors) are working on a specific chapter, so you download the manuscript that the team is keeping online. You would never edit the manuscript directly, that would be way too risky. You create a temporary version, and label it as such.

You do your work, write your lines, make sure everything is to your liking, and then what do you do?

You upload it to the Dropbox, always making sure that it's labeled correctly as a temporary version. Why? Because you would like feedback from your colleagues before pulling everything together (notice our word choice there 🧐).

What's a simple word for asking for feedback?

A review.

Dictionaries are interesting.

Anyway, your colleagues will then look at your brand new text and they will carefully look over it. They will correct mistakes, leave notes and suggestions, make recommendations, and, of course, if all is well, simply accept it.

When this is done, and the author of the new chapter is confident he's answered all the queries and fixed all the problems, they can then request whoever is responsible for the manuscript to pull his changed text into the main document.

And that is what a pull request is.

When a developer finishes a task, they will start a process to join the new code to the project. For that purpose, they will ask whoever's in charge of the project to pull the changes from their working branch to the main branch. Voilá, a request to pull changes: a pull-request.

So it's not a review, but it implies a review?

Most of the time, yes.

Unless the changes are very trivial, it would be irresponsible to pull changes into the main branch without performing a code review.

What happens frequently is the opposite: code reviews happen all the time without the need for a pull request.

A developer might want to review the changes between different states of a project for many different purposes:

Auditing the source code is always important;
Reviewing code without making it visible to everyone else in the project (pull-requests are always visible), for whatever reason;
Reviewing the code when the project is scattered across many repositories;
There are even some software development methodologies that don't use pull requests at all! And they still review code.

We hope this helped clear up what pull requests are, and why they are often wrongly mistaken for code reviews. Are there any topics you'd like us to cover next on this series?

If so, you know where to find us on Twitter (@reviewpad)!

When You Merge Pull Requests You Lose Knowledge

Marcelo Sousa — Tue, 24 Aug 2021 15:33:30 +0000

What happens to comments and discussions of a pull request when it is merged? Isn’t that knowledge valuable for the team? Why isn’t it readily accessible to future pull requests?

Pull requests (PRs) are a mainstream way of proposing changes to a codebase. One of the most important features around pull requests is the ability to perform a code review, allowing developers to inspect and comment on the proposed changes.

Frequently, the knowledge in these comments is invaluable to the team. Reviewers, which are familiar with the codebase and related technologies, will notice bad patterns, suggest optimisations, enforce team guidelines and best practices, etc.

However, as soon as you merge that pull request on GitHub, Gitlab, or Bitbucket, all this information is lost into the bag of closed PRs that you will rarely (if ever) inspect.

We all have had that déjà vu feeling when writing a comment in a code review: “Didn’t I write this comment already?” When preparing a PR sometimes we also have that feeling: “I think I’m forgetting something in this line of code.” Or better yet: “Didn’t someone already thought of this before?”

Let’s look at three scenarios.

1. Knowledge useful to a broader audience

Knowledge that could be beneficial to developers not involved in this codebase. For instance, this comment could be useful for other developers modifying similar code.

Discussion by kgabryje at apache / superset “feat(native-filters): add search all filter options #14710“

2. Knowledge about design decisions

Teams with a lot of contributors can suffer from problems associated with the lack of context in design decisions. For instance, when a decision was taken on a discussion on a previous pull request. Consider the following discussion:

Discussion with halspang and artursouza at dapr / dapr “Write reminders in multiple partitions #3297“

Assuming that this PR is merged, when someone else modifies this code in the future, it would be useful to retrieve this discussion. In this case, that would be particularly relevant if this person starts the support for a decrementing path.

3. Review déjà vu

Senior developers that are active reviewers also experience what we call review déjà vus. These are situations where you are repeating similar comments in different PRs, like this one:

Comment by timotheecour at nim-lang / Nim “oids: switch from PRNG to random module #16203“

These situations also occur when there are some hidden assumptions that do not make sense to document in the code.

Wouldn’t it be nice if we could re-use the comments from past reviews into future PRs without having to maintain them?

Enter Reviewpad

Reviewpad builds semantic diffs of changes in pull requests and associates metadata such as comments to these semantic objects. This is a big shift from the current behavior of existing code hosts such as Github, Gitlab, and Bitbucket. They associate comments to a line of a file in a particular git diff – as soon as the contents of the line change, this comment is marked as outdated.

We’ll showcase Reviewpad with the Web Crawler exercise of A Tour of Go using the linked GitHub repository. In this exercise of A Tour of Go, we are given a first implementation of a fake web crawler – the exercise is to parallelise it and prevent fetching the same URL twice.

Semantic git diffs

As a first step, we used Reviewpad to create a pull request with this first implementation.

Reviewpad interface for pull request after review and merge

Reviewpad performs a relational static analysis (in the style of [1] and [2]) over the git diff associated with the pull request to build semantic diffs. These semantic diffs form what we call the Explore Tree in Reviewpad.

Explore tree for pull request

An explore tree is an annotated file tree of the modified files. For each modified file, we present an annotated semantic tree of the changes. For example, the file fakeFetcher.go as presented on GitHub requires an entire scan to understand the main symbols added: 1) the type fakeFetcher, 2) the struct fakeResult with two fields body and urls and 3) the method Fetch. Note that the method Fetch is shown as a child of the type fakeFetcher as it is the receiver type. At the moment, our analysers focus on types and methods – this is why we don’t show variable fetcher. The color in the beginning of the symbol indicates the type of modification: green for added, red for deleted and yellow for modified.

We show the explore tree on the left of the description and general conversation so that developers can quickly match the description and discussions to code changes.

Each element of the explore tree is clickable to its representation in the git diff. For example, clicking on the fakeFetcher type directs the user to the beginning of the symbol in the diff:

Navigation from the explore tree to the code diff

Note: For those you noticed the coloured icon next to the file go.mod – this means that the file is automatically hidden from the review mode. Each Reviewpad user can tweak their file view in their review settings to exclude files.

In this PR, I did a first review to document parts of the code. For example, while the original code for Crawl is:

// Crawl uses fetcher to recursively crawl
// pages starting with url, to a maximum of depth.
func Crawl(url string, depth int, fetcher Fetcher) {
    // TODO: Fetch URLs in parallel.
    // TODO: Don't fetch the same URL twice.
    // This implementation doesn't do either:
...
}

I chose to comment on the function as a code diff comment. These review comments are inserted into the knowledge base maintained by Reviewpad:

Diff comment in Crawl function

Note that the comment in the explore tree appears as a child of Crawl symbol. This means that the comment was made in the definition of the symbol (not just in its signature).

Comments from past reviews

After the PR with the initial code was squash and merged, I created a new PR with a solution presented by the GitHub user harryhare.

Reviewpad interface for pull request with first solution

The explore tree again shows the overview of the solution which involve the introduction of a SafeCounter struct with a map v to check if an URL was visited, a mutex mux and a method checkvisited that checks if URL was already visited or not.

The rest of the solution involves changes to the functions Crawl and main to fetch URLs in parallel. In the Crawl method, Reviewpad presents the comment from the past PR that documents the method. This way, developers can retrieve discussions from previous PRs that changed this method.

Code diff view with comment from previous pull request

A user review raised an issue with the checkvisited which is fixed in the commit [fix from review](https://github.com/reviewpad/dejavus/pull/3/commits/a629a38fddad2305a13f2ce07da6a001c263920b).

A final review approves the PR with the request to remove the TODOs in the code.

In the final commit of this PR, we add the reference to the solution as a code comment in the Crawl method and remove the TODOs.

Reviewpad interface for all changes in pull request

Hovering on the function Crawl in the explore tree displays the code documentation associated with this function for readability.

Keeping track of comments and changes over time

At this point you might be wondering:

What happens if the comment was made in a PR that was not the exact previous one?
What happens if I start refactoring the code?

To answer these questions, let’s go over the two open PRs in our project.

The first is an improvement proposed by another Github user that uses wait groups.

Pull request proposing an improvement using wait groups

The comment shown in this pull request was the one made in the first PR. Reviewpad tracks comments regardless of their time difference. For example in the following PR from the google/error-prone project:

Reviewpad review mode for pull request of the google/error-prone project. You can check it on Github here.

The PR was opened in September 2020 and it is changing class methods that were commented in January 2020.

The second open pull request demonstrates a beta version of our refactoring analyser:

Pull request with a refactoring

In this PR, we introduced two commits, the first that changes the signature of the function Crawl by renaming the variable url to reqUrl and the second renames the file.

Reviewpad understands that it is the same function and presents the comment made in the first PR. Note that we are squash and merging these pull requests – the commit associated with the comment is not an ancestor of the commits in this PR.

Cool. How do I check this out by myself?

We have a public beta version of Reviewpad available at https://reviewpad.com/get-started/. You will need to create a new account and once you log in for the first time, you will see the following page to connect to a code host:

Connect to code host page on Reviewpad

You can connect to GitHub through our OAuth app or manually add a personal access token. The OAuth requires minimal scopes to be able to read and comment on public repositories.

And that’s it – you will be able to give Reviewpad a spin on public repositories! We will be adding more and more public repositories in the upcoming days – feel free to reach us on our community Slack with requests!

References

[1]: Cartesian hoare logic for verifying k-safety properties.

[2]: Verified three-way program merg e.

A code review checklist from Reviewpad

Reviewpad — Fri, 20 Aug 2021 09:25:02 +0000

Not everyone will need a checklist for code reviews, and not all code reviews will fit into your checklist. We accept that, and we have written about how some more senior developers sometimes even think they get in their way. They are a very good idea, however, if you haven't yet reached a comfort zone with your reviews.

A good checklist will keep you consistent. It will also prevent mistakes and omissions.

Here's our humble proposal, just to get you started:

Are all requirements met? 📋

Requirements are the basis for all development. Regardless of what the project you're working on is, and its scope, you will have a series of basic (and not so basic) requirements that the code must accomplish.

Check them, one by one.

Find the relevant code.

Review it.

Job done.

Are code conventions being followed? 🤝

Code conventions aren't bylaws, and they will change from project to project, and from language to language. They must be kept consistent throughout, otherwise, you can give rise to a whole ocean of troubles.

Whatever's been established for the project and language you're working with, it's your job while doing a code review, that everything is up to snuff. That ranges from things as trite as indentation to things as meaningful as comments.

Consistency is the key word.

Are magic values/scenarios accounted for? 🦄

What would you think if you saw the number 259200000 in the middle of a particularly important piece of code about dates (i.e. days and months, not romantic liaisons), with no explanation?

And that number is absolutely key for the way the function works?

You wouldn't be happy, would you? Well, that's the number of milliseconds in three days, and some people might immediately get it and others might not. Anything that is key for the inner workings of your code that isn't explained, is said to just "magically" work.

There's no magic in development. Make sure everything is clear. Which leads us to:

Is the code easy to read? 🤓

Remember: you don't know who is going to be doing maintenance on the code base your team is working on. It may be you, or not. And even if it is you, it's going to be the future you. We can promise you right now that you will not remember why that wonderfully clever function you've just written down works.

Everything needs to be readable. As readable as you can possibly make it.

Does it have good test coverage? ✅

As much as the codebase as possible should be covered when running your tests. If the percentage isn't high enough, then you probably need to consider changing the battery of tests you're running.

Tests aren't technically a part of code reviews, but they definitely go hand in hand as two of the most effective practices to ensure code quality.

Is there no unused code? 💤

Pretty self-explanatory, right? If the code is unused, it shouldn't be there. No good can come of it. Only mistakes.

Did you enjoy our little checklist? Did we miss something? Is there a key step to your reviewing process that you feel should be on everyone's list?

Make sure to drop us a line on Twitter (@codereviewpad) and let us know.