Blog User Guide Documentation API Login Register

Documentation > Getting Started

Reviewing

Last updated February 16, 2024

Anyone who has some basic familiarity with Laravel and PHP can review bug reports and pull requests. You don't need to be an expert to help.

Why Reviewing Is Important

Reviews are essential to keeping contributions in line with our standards. This keeps everyone responsible for what they contribute and put in. And lastly, since there are many more pull requests and bug reports than there are members in the core team it helps spread the burden of maintenance across the team.

There are two "types" of contributions that need to be reviewed:

  • Bug Reports: Bug reports need to be checked for completeness. Is any important information missing? Can the bug be easily reproduced?
  • Pull Requests: Pull requests contain code that fixes a bug or implements new functionality. Reviews of pull requests ensure that they are implemented properly, do no introduce new bugs, and maintain backward compatibility.

Be Constructive

Before you begin, remember that you are looking at the result of someone else's hard work. A good review comment thanks the contributor for their work, identifies what was done well, identifies what should be improved and suggests a next step.

The Bug Report Review Process

A good way to get started with reviewing is to pick a bug report. The steps for the review are:

  1. Is the Report Complete? Good bug reports should contain as much relevant information and code samples to reproduce the bug.

  2. Reproduce the Bug Create a local fork of the reported version of FusionCMS and reproduce the bug following the steps provided.

  3. Update the Bug Report Status At last, add a comment to the bug report. Thank the reporter for reporting the bug. Set the label to one of the following:

    • Needs Work If the bug does not contain enough information to be reproduced, explain what information is missing.
    • Works for me If the bug does contain enough information to be reproduced but works on your system, or if the reported bug is a feature and not a bug, provide a short explanation.
    • Reviewed If you can reproduce the bug, this will signify a contributor to tackle the issue at hand.

The Pull Request Review Process

The process for reviewing pull requests (PRs) is similar to that for bug reports. Reviews of pull requests usually take a little longer since you need to understand the functionality that has been fixed or added and find out whether the implementation is complete.

It is okay to do partial reviews! If you do a partial review, comment how far you got and leave the PR in "Needs Review" state.

  1. Is the PR Complete? Every pull request must contain a header that gives some basic information about the PR.

  2. Is the Base Branch Correct? GitHub displays the branch that a PR is based on below the title of the pull request. Is that branch correct? Unless you have a very rare case in needing to submit a patch to the master branch, all pull requests should be directed to the develop branch to be included in the upcoming release.

  3. Review the Code Read the code of the pull request and check it against some common criteria:

    • Does the code address the issue the PR is intended to fix/implement?
    • Does the PR stay within scope to address only that issue?
    • Does the PR contain sufficient comments to easily understand its code?
    • Does the code break backward compatibility? If yes, does the PR header say so?
    • Does the PR contain deprecations? If yes, does the PR header say so? Does the code contain logger()->warning() statements for all deprecated features and correct Docblocks?
  4. Update the PR Status At last, add a comment to the PR. Thank the contributor for working on the PR. Set the label to one of the following:

    • Needs Work if the PR is not yet ready to be merged, explain the issues that you found and add this label.
    • Reviewed if the PR satisfies all the checks above, add this label. From here you are free to squash and merge the PR if you have the necessary permissions - otherwise, a core contributor will soon look at the PR and merge it in.