From a319a3801848da136038237400e2a4e262bd6027 Mon Sep 17 00:00:00 2001 From: Sarah Gibson Date: Wed, 24 Jul 2024 09:26:08 +0100 Subject: [PATCH] Cross-link review and merge guidelines from instrastructure docs --- engineering/reviewing.md | 37 +------------------------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/engineering/reviewing.md b/engineering/reviewing.md index cc890cd6..02adc051 100644 --- a/engineering/reviewing.md +++ b/engineering/reviewing.md @@ -1,39 +1,4 @@ (development:merge-policy)= # Merge and Code Review policy -The sections below describe major policies around merging and reviewing depending on the kind of change being made. -There are some extra policies for changes that **affect actively-running infrastructure**. -See the section below for details. - -Not all of them are followed strictly, though some are more important than others, and are marked with **REQUIRED**. - -- **Always make a Pull Request**. (REQUIRED). - All changes to 2i2c repositories should be made via a Pull Request. - This should be enforced by setting the `main`/`master` branch of each repository to be "protected". -- **PRs should reference (and close) issues**. - A pull request should almost always be related to an issue. - Ideally, the issue should be tightly-scoped enough that the PR will close it when merged. - If you have an idea that *doesn't* yet have an issue, open an issue first and then make the PR to close it. - This ensures that the team has context around Pull Requests, and a chance to discuss before we implement. -- **Use GitHub's `Request Review` feature**. - Add specific team members so that it is clear who is needed to review the PR. -- **Be explicit about what feedback you want**. - When you open a PR, include some language about specific things you'd like feedback with, if applicable. - This helps others focus their attention. -- **Use the review column on sprint boards**. - When a PR needs review, move any relevant issues to the {kbd}`Review` column of the active [Sprint Board](coordination:deliverables-backlog) so others notice it. -- **Merge after one approval**. - If there is at least one approval on a PR, then anybody, including the PR author, may merge the PR. - PR authors should not hesitate to merge their own PR after an approval if they think it is ready to go! -- **Merging without review is discouraged, but not forbidden**. - For changes that are minor, very straightforward, and do not affect actively-running infrastructure, it is acceptable to self-merge a PR without getting an approval. - If you don't believe that your PR requires an approval before merging, make it clear in your PR or in a comment that you plan to merge it in 24 hours. -- **Leave PRs open for at least 24 working hours**. - This helps ensure that others on the team have a chance to look at the PR and give their thoughts (by working hours we mean hours during a weekday). - - -## Policy for changes to running infrastructure - -Changing active infrastructure is a bit different from developing technology that is not immediately in production. -As such, we follow some more specific guidelines for these kinds of changes. -See {external+infra:ref}`infrastructure:review`. +[See the Review and Merge Guidelines document for 2i2c engineers in the infrastructure documentation.](https://infrastructure.2i2c.org/contributing/code-review/)