Managing Pull Requests

Reviewing a pull request can take a considerable amount of time. In some cases, the review might require more time to perform than it took someone to write and submit their changes! It's therefore necessary to do some preliminary work to ensure each pull request is in a good state to be reviewed.

A pull request should consist of three main sections:

• A summary. This helps us understand the motivation behind the changes.
• A changelog. This helps us write the release notes. It also serves as a brief summary of your changes.
• A test plan. This might be the most important part of your pull request. A test plan should be a reproducible step-by-step guide so that a reviewer can verify your change is working as intended. It's also a good idea to attach screenshots or videos for user visible changes.

Any one pull request may require a deeper understanding of some area of React Native that you may not be familiar with. Even if you don't feel like you are the right person to review a pull request, you may still help by adding labels or asking the author for more information.

Reviewing PRs

Pull Requests need to be reviewed and approved using GitHub's review feature before they can be merged. While anyone has the ability to review and approve a pull request, we typically only consider a pull request ready to be merged when the approval comes from one of the contributors.

So you've found a pull request that you feel confident reviewing. Please make use of the GitHub Review feature, and clearly and politely communicate any suggested changes.

Consider starting with pull requests that have been flagged as lacking a changelog or test plan.

• PRs that appear to lack a changelog - take a look and see if you can add the changelog yourself by editing the PR. After doing so, remove the "Missing Changelog” label.
• PRs that are missing a test plan - open the pull request and look for a test plan. If the test plan looks sufficient, remove the "Missing Test Plan” label. If there is no test plan, or it looks incomplete, add a comment politely asking the author to consider adding a test plan.

A pull request must pass all the tests before it can be merged. They run on every commit on development and pull request. A quick way to help us get pull requests ready for review is to search for pull requests that are failing the pre-commit tests and determine if they need to be revised. The failing test is usually listed near the bottom of the thread, under “Some checks were not successful.”

How we prioritize PRs

Members of the Core Engineer team aim to review pull requests quickly and most PRs will get a response within a week.

Pull Request Labels

• Merged: Applied to a closed PR to indicate that its changes have been incorporated into the core repository. This label is necessary because pull requests are not merged directly on GitHub. Instead, a patch with the PR's changes is imported and queued up for code review. Once approved, the result of applying those changes on top of OsmiCSX internal monorepository gets synced out to GitHub as a new commit. GitHub does not attribute that commit back to the original PR, hence the need for a label that communicates the PR's true status.