From c3580769fb402db0167499753f88441312506217 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 19 Oct 2023 07:38:09 +1000 Subject: [PATCH 1/4] Document branch protection rules --- book/src/dev/continuous-integration.md | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/book/src/dev/continuous-integration.md b/book/src/dev/continuous-integration.md index 895085f6395..79b60622674 100644 --- a/book/src/dev/continuous-integration.md +++ b/book/src/dev/continuous-integration.md @@ -33,10 +33,25 @@ We try to use Mergify as much as we can, so all PRs get consistent checks. Some PRs don't use Mergify: - Mergify config updates - Admin merges, which happen when there are multiple failures on the `main` branch -- Manual merges + (these are disabled by our branch protection rules, but admins can remove the "don't allow bypassing these rules" setting) +- Manual merges (these are usually disabled by our branch protection rules) We use workflow conditions to skip some checks on PRs, Mergify, or the `main` branch. -For example, some workflow changes skip Rust code checks. +For example, some workflow changes skip Rust code checks. When a workflow can skip a check, we need to create a patch workflow +with an empty job with the same name. This lets the branch protection rules pass when the job is skipped. In Zebra, we name these +workflows with the extension `.patch.yml`. + +Branch protecion rules should be added for every failure that should stop a PR merging, break a release, or cause problems for Zebra users. +We also add branch protection rules for developer or devops features that we need to keep working, like coverage. + +But the following jobs don't need branch protection rules: +* Testnet jobs: testnet is unreliable. +* Optional linting jobs: some lint jobs are required, but some jobs like spelling and actions are optional. +* Jobs that rarely run: for example, cached state rebuild jobs. +* Setup jobs that will fail another later job which always runs, for example: Google Cloud setup jobs. + We have branch protection rules for build jobs, but we could remove them if we want. + +When a new job is added in a PR, use the `#devops` Slack channel ask a GitHub admin to add a branch protection rule after it merges. ### Pull Requests from Forked Repositories From 5e94eb756ce59ebcccc0b4aced069870e766b486 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 19 Oct 2023 07:40:44 +1000 Subject: [PATCH 2/4] Document automatic jobs for new crates --- book/src/dev/continuous-integration.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/book/src/dev/continuous-integration.md b/book/src/dev/continuous-integration.md index 79b60622674..7884d2cd36c 100644 --- a/book/src/dev/continuous-integration.md +++ b/book/src/dev/continuous-integration.md @@ -52,6 +52,8 @@ But the following jobs don't need branch protection rules: We have branch protection rules for build jobs, but we could remove them if we want. When a new job is added in a PR, use the `#devops` Slack channel ask a GitHub admin to add a branch protection rule after it merges. +Adding a new Zebra crate automatically adds a new job to build that crate by itself in [ci-build-crates.yml](https://github.com/ZcashFoundation/zebra/blob/main/.github/workflows/ci-build-crates.yml), +so new crate PRs also need to add a branch protection rule. ### Pull Requests from Forked Repositories From af57dc5c8a24457833c3aeece58ae455e9bcaf12 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 20 Oct 2023 06:55:14 +1000 Subject: [PATCH 3/4] Link to patch workflow docs --- book/src/dev/continuous-integration.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/book/src/dev/continuous-integration.md b/book/src/dev/continuous-integration.md index 7884d2cd36c..2ec910c0428 100644 --- a/book/src/dev/continuous-integration.md +++ b/book/src/dev/continuous-integration.md @@ -37,11 +37,11 @@ Some PRs don't use Mergify: - Manual merges (these are usually disabled by our branch protection rules) We use workflow conditions to skip some checks on PRs, Mergify, or the `main` branch. -For example, some workflow changes skip Rust code checks. When a workflow can skip a check, we need to create a patch workflow -with an empty job with the same name. This lets the branch protection rules pass when the job is skipped. In Zebra, we name these -workflows with the extension `.patch.yml`. +For example, some workflow changes skip Rust code checks. When a workflow can skip a check, we need to create [a patch workflow](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#handling-skipped-but-required-checks) +with an empty job with the same name. This is a [known Actions issue](https://github.com/orgs/community/discussions/13690#discussioncomment-6653382). +This lets the branch protection rules pass when the job is skipped. In Zebra, we name these workflows with the extension `.patch.yml`. -Branch protecion rules should be added for every failure that should stop a PR merging, break a release, or cause problems for Zebra users. +Branch protection rules should be added for every failure that should stop a PR merging, break a release, or cause problems for Zebra users. We also add branch protection rules for developer or devops features that we need to keep working, like coverage. But the following jobs don't need branch protection rules: @@ -60,7 +60,7 @@ so new crate PRs also need to add a branch protection rule. GitHub doesn't allow PRs from forked repositories to have access to our repository secret keys, even after we approve their CI. This means that Google Cloud CI fails on these PRs. -Unril we [fix this CI bug](https://github.com/ZcashFoundation/zebra/issues/4529), we can merge external PRs by: +Until we [fix this CI bug](https://github.com/ZcashFoundation/zebra/issues/4529), we can merge external PRs by: 1. Reviewing the code to make sure it won't give our secret keys to anyone 2. Pushing a copy of the branch to the Zebra repository 3. Opening a PR using that branch From 596c3edc2af35a41993b103e56f8833e8fddc3a4 Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Sun, 22 Oct 2023 14:46:08 +0100 Subject: [PATCH 4/4] Update book/src/dev/continuous-integration.md Co-authored-by: Marek --- book/src/dev/continuous-integration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/book/src/dev/continuous-integration.md b/book/src/dev/continuous-integration.md index 2ec910c0428..bd9db0ae2e7 100644 --- a/book/src/dev/continuous-integration.md +++ b/book/src/dev/continuous-integration.md @@ -51,7 +51,7 @@ But the following jobs don't need branch protection rules: * Setup jobs that will fail another later job which always runs, for example: Google Cloud setup jobs. We have branch protection rules for build jobs, but we could remove them if we want. -When a new job is added in a PR, use the `#devops` Slack channel ask a GitHub admin to add a branch protection rule after it merges. +When a new job is added in a PR, use the `#devops` Slack channel to ask a GitHub admin to add a branch protection rule after it merges. Adding a new Zebra crate automatically adds a new job to build that crate by itself in [ci-build-crates.yml](https://github.com/ZcashFoundation/zebra/blob/main/.github/workflows/ci-build-crates.yml), so new crate PRs also need to add a branch protection rule.