Skip to main content

Code review

r
package development
Why it is important to read other people’s code and how to do it.
Authors

Bethany Allen

Etienne Bacher

Lewis A. Jones

Published

November 11, 2026

View source file

install.packages(c("palaeoverse"))
NoneLearning Objectives

After completing this module, you will:

  • Understand what code review is and its benefits
  • Be able to perform self code review
  • Be able to review other people’s code, and clearly and appropriately communicate your feedback
  • Be able to use GitHub features for code review

Motivation

We spend a lot of time writing code, but even more time reading it. This could be code that is part of a tutorial, a package, some analysis shared by a colleague, or even when we come back to our own code months later. Being able to read and understand code before running it is important, as we may not be able to immediately run code (for instance if it requires an internet connection), or we may have to wait for minutes or hours before the execution ends.

Being able to review code becomes even more important as the amount of AI-generated code keeps growing. As AI gets better at writing code, we spend more and more time reviewing code to ensure its correctness than writing it ourselves.

Finally, reviewing code is a good way to learn about new methods, new coding practices, and new functions that you might not have known existed.

How to review code

The main objectives of code review are to:

  1. verify that the code aligns with what we expect it to do;
  2. find potential mistakes;
  3. ensure that the code is maintainable, meaning that it follows good practices so that it can be understood, fixed, or improved in the future.
NoteExpectations for code authors and code reviewers

Both authors and reviewers should have realistic expectations of each other’s roles.

The code author is responsible for the code they are requesting to be reviewed, including enacting any change proposed during the process. They should do their best to follow good practices in order to facilitate code review and to ensure that other people can understand the code without asking them. This includes using informative variable names, providing clear documentation, and writing informative code comments to clarify why a piece of code is written how it is.

The reviewer’s role is to give opinions on the code from an alternative point of view to the author. This might require gathering some external knowledge that the author doesn’t necessarily have to provide. For example, if the reviewer is not familiar with a base R function that has been used, then they may need to learn this information on their own. The author is not responsible for teaching the reviewer about this function, but they should ensure that the reviewer can easily retrieve all the required information to understand the code.

This may add too much burden on the reviewer, in which case it might be better to transparently acknowledge that they cannot review some code (and, if possible, they may request a review from someone else). This is not ideal but it’s also not the end of the world. Also, finding and resolving bugs can be a beneficial part of the review process, but remember that bugs can persist in code that has been reviewed by many people!

Depending on the setting, the reviewer might start by checking that the code actually runs, but this is not always possible or needed (for instance if there are already automated tests that ensured that the code was giving the correct results).

The reviewer should then ask the following questions: what does this code do? What are the inputs and outputs? Do I have enough information (e.g. code comments, informative variable names) to understand that? Is there anything that is hard to get and not documented well enough? Is the code human-readable?

Once the reviewer is more familiar with the code, they can follow up with questions such as: what could go wrong? What are the assumptions, and what would happen if some of these break? Are these cases realistic? Are these cases properly handled?

A good way to assess your own understanding of the code is to prepare a list of expectations about what this code should do, and then compare these expectations to what the code actually does. This is not always convenient or possible, for example if the code to review is part of a pipeline that takes a long time to run, but it can uncover issues in many cases.

Note that the amount of tests or expectations to validate is also very context-dependent. In general, code written as part of a data analysis project is very specific to your use case and its purpose is not to run on completely different datasets. Code that is part of a data analysis project still needs to be properly reviewed and tested, but you have more control over how this code is used. If your project is a package meant to be used by other people, then you also need to think about all sorts of ways people might misuse your code in their own context.

Activity: Try out reviewing mini “bad code” examples that we provide

CautionExercise 1

Without running these examples, do you see what is likely wrong or could be improved?

library(palaeoverse)

x <- tetrapods[1:3, c("collection_no", "collection_name", "lng")]
y <- tetrapods[1:3, c("collection_no", "collection_name", "formation")]
out <- merge(x, y, by = "collection_no")
out[["collection_name"]]

out[["collection_name"]] returns NULL because we have "collection_name.x" and "collection_name.y" in the output, because we forgot to merge on "collection_name" as well:

x <- tetrapods[1:3, c("collection_no", "collection_name", "lng")]
y <- tetrapods[1:3, c("collection_no", "collection_name", "formation")]
- out <- merge(x, y, by = "collection_no")
+ out <- merge(x, y, by = c("collection_no", "collection_name"))
out[["collection_name"]]
x <- head(tetrapods$max_ma)
x_updated <- c()

for (i in seq_along(x)) {
  if (x[i] > 270) {
    x_updated[i] <- x[i] * 0.9
  } else {
    x_updated[i] <- x[i]
  }
}

In general, we should prefer vectorised code to for loops where possible. In this case, the function ifelse() is preferable to using an if condition in a loop because it can take the whole vector at once:

x_updated <- ifelse(x > 270, x * 0.9, x)
x <- head(tetrapods$min_ma, 3)
idx_to_remove <- x[x < 0]
x[-idx_to_remove]

In this case, we want to remove some values of x based on some index that we computed. The problem here is that if idx_to_remove is of length 0 because x doesn’t have negative values, then x[-idx_to_remove] returns an empty vector instead of returning the entire vector!

x[-idx_to_remove]
numeric(0)

A safer way to do that is to use an if condition:

if (length(idx_to_remove) > 0) {
  x[-idx_to_remove]
}

Where to review code

Now that we know how to critically review code, we need to be able to communicate our findings. We could talk directly, or via email or a messaging service, but that quickly leads to information being lost, which increases the chances of these issues reappearing later. Instead, we should use tools that are dedicated to collaborative coding, such as Git and GitHub.

In this section, we assume that you are already familiar with the Git workflow to create and merge branches. We also assume that you have a GitHub account. The Git and GitHub module provides an introduction to these two tools.

How to use the GitHub interface to review pull requests (PRs)

  • Go to the PR
  • Go to the tab “Files changed”. All changes are displayed there. Some files are collapsed (e.g. Rd files), indicating that they are automatically generated and shouldn’t be reviewed.
  • When you hover over a line, a blue cross appears on the left. This is how you can select the line for which you want to add a comment.
  • Write your comment using the Markdown syntax. There is a button “Add a suggestion” that you can use to directly show the change you would like the author to apply.
  • All comments you make are “Pending”, meaning that they are not visible to the author until you submit your review. They are still saved, so you can leave without submitting your review and come back to it later if needed.
  • Once you have written all your comments, go to “Submit review”. There, you can decide whether you want to accept the PR, request changes (meaning that the author won’t be able to merge until you approve), or comment only. You can also write a general message regarding the PR.
  • Once you are happy with your comments, you can submit your review.

Note that you can pull the branch locally to directly try it and explore more before writing your review. However, depending on the amount and on the complexity of changes, reviewing on GitHub only without actually running code is completely fine.

Self-reviewing your code

Before asking someone else to review the code, it is a good practice to review your code yourself. This largely overlaps with what a reviewer would do, meaning that you need to be able to answer the questions detailed in the section “How to review code?” above. Try to put yourself in the position of someone who knows less about your project than you do, is everything clearly explained?

You also need to ensure that your code is being tested. In an R package, this means that the test suite must not have any failures (you may also need to add new tests). In a data analysis project, this could be done via data validation (e.g. with validate), as well as with tests.

Finally, you should make sure your code to be reviewed is as self-contained as possible. It is good practice to have one pull request (PR) per new analysis, feature or bug fix: it is easier and faster to review (which benefits both the author and the reviewer) and it helps debugging in the future, since it is easier to narrow down the change that caused a problem. In many cases, PRs should contain code changes, updated documentation, and new tests, but this depends a lot on the context.

Review process

Reviewer side

Communicating comments to the author in a constructive way can be hard. At the outset, the reviewer should remember the overarching goal of the code they are reviewing. For example, a suggestion which improves performance at the expense of readability may conflict with the goals of the PR or project. This context will determine the scope and relevance of the comments needed.

It is important that the reviewer focuses on critiquing the code and not the author. Alienating the author has no benefit in terms of output and morale: either the discussion becomes more aggressive and the code isn’t properly written or reviewed anymore, or one of the parties involved simply stops spending time on the PR, and maybe will stop contributing altogether.

When the reviewer disagrees with the approach taken by the author in a part of the code, they should explain why they think another approach would be more suitable. You can also include code suggestions directly using the GitHub interface. Note, however, that reviewers do not have to specify exactly how they would do this and they don’t have to provide code directly. In general, it is up to the PR author to address comments.

For example:

  • Bad: “You shouldn’t use a for loop here, this is obviously very slow.”
    • Diminishes the author by using “obviously”
    • Doesn’t suggest a solution.
  • Better: “Can we use a vectorised function instead? This is often much faster than for loops.”
    • Uses “we” to emphasise team work
    • Gives the opportunity to the author to explain why they wrote the code as they did (maybe they made a mistake, maybe not).
    • Gives a potential alternative (but the exploration still needs to be done by the author)

Note that positive comments are welcome to acknowledge good changes. Remember that even if you think the author did a good job, they will mostly see review comments that criticise their work, which can be demoralising.

Author side

Before requesting a review, the author should make sure that automated checks pass. These can include package tests, data validation checks, and linting and styling checks, among other things. It is the author’s responsibility to ensure that these checks pass, the reviewer shouldn’t have to ask for them to be fixed (but the author can ask for help if they cannot fix some failures).

On top of preparing a self-contained PR and doing some self-review of their code before requesting a review (see above), the author can take several actions to simplify the review process and make it more efficient.

First, the author should succinctly describe the objective of the PR: does it improve the documentation? Does it fix a bug? Does it add a new feature? If so, is this feature a new function, a new argument, something else? And so on. This doesn’t have to be a long description, one or two sentences may be enough. Note that it is also good practice to link to the issue addressed by this PR. You can add “Closes #1234” or “Fixes #1234” so that issue 1234 is automatically closed when the PR is merged.

It can be helpful to describe specific goals for the review process, for instance if you want help with the efficiency of your code. The PR author can also add their own review comments using the GitHub interface before requesting a review. Some parts of a PR might be hard to understand without the context, so it is completely fine for the author to add a few comments to clarify some specific lines of code in advance. The author can also use these comments to inform the reviewer of specific lines for which they have doubts and would like feedback on some of their changes. Note that review comments on GitHub can be displayed in a variety of ways, which can make them easier to identify and read than writing comments directly in the file.

The author should address all comments and questions of the reviewer. This doesn’t mean agreeing with the reviewer all the time, but disagreements should be based on concrete arguments. Handling the reviewer’s comments can be a frustrating process, but it is important to stay polite and to ground opinions on facts and concrete examples regarding the code. The review process is asynchronous, so you usually don’t need to reply instantly to the reviewer and you can instead take some time to think more about their points.

If you keep disagreeing with the reviewer, it might be better to discuss in person to avoid misunderstandings. You can seek external opinions in the case of strong disagreement, but this depends on the size of your team.

Discussion points between authors and reviewers

Some practices vary across developers and it’s hard to tell whether some are better than others. Usually, these can be cleared up by asking the other person directly what they prefer, or they can be integrated in a pull request template or a project-wide policy that the author has to follow.

One of these discussion points is about “resolving comments”. On GitHub, one can hide a comment by “resolving” it, but it is not clear whether both the author and the reviewer should resolve comments, or if only the reviewer should do this. A reviewer might want to check that all their comments are properly addressed before marking them as resolved, but some reviewers are also fine with letting PR authors resolve comments themselves.

Another discussion point concerns who should push commits to the branch. Usually, both the author and the reviewer have the permissions to push to the same branch, but this can lead to conflicts. This is also worth clarifying upfront, particularly as a reviewer who pushes to the branch technically also becomes a co-author of the code, and this may have implications for how credit is attributed. It can be tiring for the PR author to go through multiple rounds of review, so in this case it is often acceptable (but not required) for the reviewer to fix the few remaining minor changes to make (e.g. typos) before merging the PR to avoid another round of review.

Finally, AI usage might be a point of contention, in which case it is important that both the author and the reviewer agree on the acceptable AI usage, both in code and in discussion. Sometimes, this is covered by a project-wide AI policy, but in any case it is worth being transparent about this.

Summary: a checklist for authors and reviewers

Authors

Before requesting a review:

  • Does the PR contain multiple types of changes that should be in separate PRs instead?
  • Have you reviewed, tested, and documented the changes (if necessary)?
  • Do all automated checks pass?
  • Is the PR description a clear summary of the changes included in the PR? Have you linked to related issues in the PR description? If relevant, have you added “auto-close” comments (e.g. “Close #1234”)?

If you are in charge of merging the PR after the review:

  • Have you updated the PR description after addressing all the review comments (if needed)?
  • Have you left some review comments “unresolved”? If so, have you explained why?

Reviewers

Questions to consider and potentially to report during the review:

  • Do the code changes align with the description in the PR (or your understanding of what the changes aim to achieve)?
  • Are changes properly documented, through code annotations and formal descriptions (e.g. repository README, roxygen function documentation)?
  • Are changes properly and sufficiently tested?
  • Are there changes that should be moved to a separate PR?
  • If relevant, are these changes documented in a user-facing document (e.g. a changelog)?

Conclusion

Being able to critically review code, no matter who or what produced it, and to communicate feedback in a constructive way, are essential skills. In this module, we have seen the general principles of code review, how to review code in practice, and discussed how to facilitate communication between the author and the reviewer.

If you want to read more on the process, you can read the code review principles from the tidyverse team, themselves inspired from the Google code review principles.


Session info 
─ Session info ───────────────────────────────────────────────────────────────
 setting  value
 version  R version 4.6.2 (2026-10-29)
 os       Ubuntu 26.04.1 LTS
 system   x86_64, linux-gnu
 ui       X11
 language (EN)
 collate  C.UTF-8
 ctype    C.UTF-8
 tz       UTC
 date     2026-11-11
 pandoc   3.7.0.2 @ /usr/bin/pandoc
 quarto   1.11.9 @ /usr/local/bin/quarto

─ Packages ───────────────────────────────────────────────────────────────────
 ! package     * version date (UTC) lib source
   ape           5.8-1   2024-12-16 [1] RSPM
   askpass       1.2.1   2024-10-04 [1] RSPM
   class         7.3-24  2026-08-03 [2] CRAN (R 4.6.2)
   classInt      0.4-11  2025-01-08 [1] RSPM
   cli           3.6.6   2026-04-09 [1] RSPM
 R cpp11         <NA>    <NA>       [?] <NA>
   curl          8.1.0   2026-10-14 [1] RSPM
   DBI           1.3.0   2026-02-25 [1] RSPM
   digest        0.6.39  2025-11-19 [1] RSPM
   dplyr         1.2.1   2026-04-03 [1] RSPM
   e1071         1.7-17  2025-12-18 [1] RSPM
   generics      0.1.4   2025-05-09 [1] RSPM
   geojsonsf     2.0.5   2025-11-26 [1] RSPM
 R geometries    <NA>    <NA>       [?] <NA>
   geosphere     1.6-8   2026-04-05 [1] RSPM
   glue          1.8.1   2026-04-17 [1] RSPM
   h3jsr         1.3.1   2023-01-21 [1] RSPM
   httr          1.4.9   2026-09-01 [1] RSPM
 R jsonify       <NA>    <NA>       [?] <NA>
   jsonlite      2.0.0   2025-03-27 [1] RSPM
   KernSmooth    2.23-27 2026-08-12 [2] CRAN (R 4.6.2)
   lattice       0.23-1  2026-08-12 [2] CRAN (R 4.6.2)
   lifecycle     1.0.5   2026-01-08 [1] RSPM
   magrittr      2.0.5   2026-04-04 [1] RSPM
   mime          0.13    2025-03-17 [1] RSPM
   nlme          3.1-171 2026-09-01 [2] CRAN (R 4.6.2)
   openssl       2.5.0   2026-11-02 [1] RSPM
   palaeoverse   1.5.0   2026-06-11 [1] RSPM
   pbapply       1.7-5   2026-09-01 [1] RSPM
   pillar        1.11.1  2025-09-17 [1] RSPM
   pkgconfig     2.0.3   2019-09-22 [1] RSPM
   proxy         0.4-29  2025-12-29 [1] RSPM
   purrr         1.2.2   2026-04-10 [1] RSPM
   R6            2.6.1   2025-02-15 [1] RSPM
 R rapidjsonr    <NA>    <NA>       [?] <NA>
   Rcpp          1.1.2   2026-07-05 [1] RSPM
   rlang         1.3.0   2026-07-05 [1] RSPM
   s2            1.1.13  2026-09-28 [1] RSPM
   sf            1.1-3   2026-09-11 [1] RSPM
 R sfheaders     <NA>    <NA>       [?] <NA>
   stringdist    0.9.17  2026-01-16 [1] RSPM
   stringi       1.8.9   2026-08-04 [1] RSPM
   stringr       1.6.0   2025-11-04 [1] RSPM
   sys           3.4.3   2024-10-04 [1] RSPM
   tibble        3.3.1   2026-01-11 [1] RSPM
   tidyr         1.3.2   2025-12-19 [1] RSPM
   tidyselect    1.2.1   2024-03-11 [1] RSPM
   units         1.0-1   2026-03-11 [1] RSPM
   utf8          1.2.6   2025-06-08 [1] RSPM
   V8            8.2.1   2026-10-26 [1] RSPM
   vctrs         0.7.3   2026-04-11 [1] RSPM
   withr         3.0.3   2026-06-19 [1] RSPM
   wk            0.9.5   2025-12-18 [1] RSPM

 [1] /home/runner/work/_temp/Library
 [2] /opt/R/4.6.2/lib/R/library

 R ── Package was removed from disk.

──────────────────────────────────────────────────────────────────────────────