Skip to content

Commit

Permalink
Docs: update PR document to better reflect current workflow. (#19184)
Browse files Browse the repository at this point in the history
Co-authored-by: Brad Jorsch <[email protected]>
Co-authored-by: Sergey Mitroshin <[email protected]>
  • Loading branch information
3 people authored Mar 16, 2021
1 parent 8c47c21 commit 256f61b
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 7 deletions.
2 changes: 1 addition & 1 deletion docs/code-reviews.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Every PR should be reviewed and approved by someone other than the author, even

The recommended way of finding an appropriate person to review your code is by [blaming](https://help.github.com/articles/using-git-blame-to-trace-changes-in-a-file/) one of the files you are updating and looking at who was responsible for previous commits on that file.

Then, you may ask that person to review your code by mentioning his/her GitHub username on the PR comments like this:
Then, you may ask that person to review your code by mentioning their GitHub username on the PR comments like this:

```
cc @username
Expand Down
2 changes: 1 addition & 1 deletion docs/git-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ For example, you can run: `git checkout master` and then `git checkout -b fix/wh

The Jetpack repo uses the following "reserved" branch name conventions:

* `branch-{X.Y|something}[-built]` -- Used for release branches
* `{something}/branch-{X.Y|something}` -- Used for release branches
* `feature/{something}` -- Used for feature branches for larger feature projects when anticipated there will be multiple PRs into that feature branch.

## Mind your commits
Expand Down
12 changes: 7 additions & 5 deletions docs/pull-request.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,19 @@ Once you know what the first small piece of your feature will be, follow this ge
- If you have write access, add the **<span class="label status-in-progress">[Status] In Progress</span>** label or wait until somebody adds it. This indicates that the pull request isn’t ready for a review and may still be incomplete. On the other hand, it welcomes early feedback and encourages collaboration during the development process.
1. Start developing and pushing out commits to your new branch.
- Push your changes out frequently and try to avoid getting stuck in a long-running branch or a merge nightmare. Smaller changes are much easier to review and to deal with potential conflicts.
- Don’t be afraid to change, [squash](http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html), and rearrange commits or to force push - `git push -f origin fix/something-broken`. Keep in mind, however, that if other people are committing on the same branch then you can mess up their history. You are perfectly safe if you are the only one pushing commits to that branch.
- Don’t be afraid to change, [squash](http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html), and rearrange commits or to force push - `git push --force-with-lease origin fix/something-broken`. Keep in mind, however, that if other people are committing on the same branch then you can mess up their history. You are perfectly safe if you are the only one pushing commits to that branch.
- Squash minor commits such as typo fixes or [fixes to previous commits](http://fle.github.io/git-tip-keep-your-branch-clean-with-fixup-and-autosquash.html) in the pull request.
- Remember to [respect Coding Standards & Guidelines.](coding-guidelines.md)
1. If you have [Composer installed](https://getcomposer.org/), you can run `composer install` and `vendor/bin/phpcs [directory or files updated]` to check your changes against WordPress coding standards. Many files are not fully within the standard yet, but please ensure your changes respect current coding standards.
1. If you have [Composer installed](https://getcomposer.org/), you can run `composer install` and `composer phpcs:lint [directory or files updated]` to check your changes against WordPress coding standards. Many files are not fully within the standard yet, but please ensure your changes respect current coding standards.
1. If you end up needing more than a few commits, consider splitting the pull request into separate components. Discuss in the new pull request and in the comments why the branch was broken apart and any changes that may have taken place that necessitated the split. Our goal is to catch early in the review process those pull requests that attempt to do too much.
1. When you feel that you are ready for a formal review or for merging into `master` make sure you check this list.
- Make sure your Pull Request [includes a changelog entry](writing-a-good-changelog-entry.md) for each project touched.
- Make sure all required checks (other than "Required review") listed at the bottom of the Pull Request are passing.
- Make sure your branch merges cleanly and consider rebasing against `master` to keep the branch history short and clean.
- If there are visual changes, add before and after screenshots in the pull request comments.
- Add unit tests, or at a minimum, provide helpful instructions for the reviewer so he or she can test your changes. This will help speed up the review process.
- Add unit tests, or at a minimum, provide helpful instructions for the reviewer so they can test your changes. This will help speed up the review process.
- Check [Coding Standards & Guidelines](coding-guidelines.md) one last time.
1. Mention that the PR is ready for review or if you have write access remove the **<span class="label status-in-progress">[Status] In Progress</span>** label from the pull request and add the **<span class="label status-needs-review">[Status] Needs Review</span>** label - someone will provide feedback on the latest unreviewed changes. The reviewer will also mark the pull request as **<span class="label needs-author-reply">[Status] Needs Author Reply</span>** if they think you need to change anything. You can [learn more about our code reviews here.](code-reviews.md)
1. Mention that the PR is ready for review. If you have write access, remove the **<span class="label status-in-progress">[Status] In Progress</span>** label from the pull request and add the **<span class="label status-needs-team-review">[Status] Needs Team Review</span>** label - someone on your team will provide feedback on the latest unreviewed changes. The reviewer will also mark the pull request as **<span class="label needs-author-reply">[Status] Needs Author Reply</span>** if they think you need to change anything. You can [learn more about our code reviews here.](code-reviews.md)
1. Once someone on your team has approved the changes, they'll update the labels from **<span class="label status-needs-team-review">[Status] Needs Team Review</span>** to **<span class="label status-needs-review">[Status] Needs Review</span>** so one of the Jetpack Approvers can do a final review of your Pull Request before it can be merged.
1. If your PR contains important changes and needs to be included in the next release, let us know! You can do so in the PR description, or in a comment on that PR. If you have the permissions, you can also add the **<span class="label pri-blocker">[Pri] Blocker</span>** label to the PR. You can also reach out to us directly if needed, either by mentioning one of us in the PR or via Slack if you're a member of the Automattic team.
1. If you get a **<span class="label ready-to-merge">[Status] Ready To Merge</span>** label, the pull request is ready to be merged into `master`.
1. The release manager will take a look and test your branch, and merge it into `master` so it can be part of the next release.
4 changes: 4 additions & 0 deletions docs/writing-a-good-changelog-entry.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

Part of our standard [Pull Request process](./monorepo.md#jetpack-changelogger) includes submitting a changelog entry for your changes, which this document provides guidance on.

## How do I create a changelog entry?

**[Follow the instructions here to use the changelogger tool to create a new entry.](./monorepo.md#using-the-jetpack-changelogger)**

## Do I even need to write a changelog entry?

For Jetpack, our changelog is intended primarily for end users and third-party developers who use our public APIs and packages. Other projects in the Jetpack monorepo also benefit from managing an accurate changelog.
Expand Down

0 comments on commit 256f61b

Please sign in to comment.