Skip to content

Commit

Permalink
Updated code review guidelines.
Browse files Browse the repository at this point in the history
  • Loading branch information
trmartin4 committed Jun 29, 2024
1 parent c42465b commit 20c1eb4
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 22 deletions.
10 changes: 9 additions & 1 deletion docs/contributing/pull-requests/code-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,17 @@ claims. This can be based on testing the change or on previous domain knowledge.
- Solve the intended problem,
- [solve the requirements in the best way](#assumptions-note),
- the code is well structured,
- follows our most recent, accepted patterns,
- follows our most recent, accepted patterns as defined in our [ADRs](../../architecture/adr/),
- and is free of unintended side-effects.

:::warning Evolutionary Database Design

For any database changes, be sure that they follow
[EDD](../../contributing/database-migrations/edd.mdx). This is important as the lack of EDD support
will **not** be caught by any unit, integration, or regression testing.

:::

If you are unsure about any of the above, consider using a different status or check in with the
author to discuss things first. Also don’t hesitate to request a second review from someone else.

Expand Down
42 changes: 21 additions & 21 deletions docs/contributing/pull-requests/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,33 +115,16 @@ These reviews are required for the changes to be merged to `master`.
You can tag additional teams or individuals for review as you see fit, including `@dept-design` for
any design changes.

</Bitwarden>

### Opening the PR for review

As its name implies, marking a PR as "Ready for Review" indicates that you are ready for all
assigned teams to review it. If the changes are still in progress, leave the PR in `Draft` status.
Doing this ensures that reviewers can act on the "Ready for Review" as their signal to begin the
review process without further notification.

### Addressing feedback

It is likely that you will receive some feedback on your PR. You should see this as a positive thing
-- it signifies a healthy and thorough review process and an organizational commitment to code
quality. You may receive [comments](./code-review.md#comment) or a
[request for changes](./code-review.md#request-changes). You are encouraged to engage in
conversation on the PR to discuss a solution, but if any strong conflicting opinions arise it is
often best to move the conversation to a synchronous format to avoid any misunderstanding.

When any necessary changes have been made, you should address the comments or request for changes by
responding in the PR conversation thread. You are not responsible for resolving the conversation --
that is the prerogative of the reviewer, to ensure that they agree that the question or concern has
been addressed.

**When you are ready for a reviewer to revisit your changes, you should request a re-review.** This
will notify the reviewer and ensure a prompt response.

</Bitwarden>

## Reviewing the pull request
## Managing the review process

<Bitwarden>

Expand Down Expand Up @@ -199,7 +182,24 @@ that may delay this process.

</Community>

### How to perform a review
### Addressing feedback

It is likely that you will receive some feedback on your PR. You should see this as a positive thing
-- it signifies a healthy and thorough review process and an organizational commitment to code
quality. You may receive [comments](./code-review.md#comment) or a
[request for changes](./code-review.md#request-changes). You are encouraged to engage in
conversation on the PR to discuss a solution, but if any strong conflicting opinions arise it is
often best to move the conversation to a synchronous format to avoid any misunderstanding.

When any necessary changes have been made, you should address the comments or request for changes by
responding in the PR conversation thread. You are not responsible for resolving the conversation --
that is the prerogative of the reviewer, to ensure that they agree that the question or concern has
been addressed.

**When you are ready for a reviewer to revisit your changes, you should request a re-review.** This
will notify the reviewer and ensure a prompt response.

## Performing a review

We've written up some [guidelines](./code-review.md) for reviewing code, which we recommend reading
before performing your first code review.
Expand Down

0 comments on commit 20c1eb4

Please sign in to comment.