diff --git a/docs/toml-schema.md b/docs/toml-schema.md index a8c7daf4b..a651efee9 100644 --- a/docs/toml-schema.md +++ b/docs/toml-schema.md @@ -304,13 +304,18 @@ octocat = "write" # The pattern matching the branches to be protected (required) pattern = "master" # Which CI checks to are required for merging (optional) +# Cannot be set if `pr-required` is `false`. ci-checks = ["CI"] # Whether new commits after a reviewer's approval of a PR # merging into this branch require another review. # (optional - default `false`) dismiss-stale-review = false +# Is a PR required when making changes to this branch? +# (optional - default `true`) +pr-required = true # How many approvals are required for a PR to be merged. # This option is only relevant if bors is not used. +# Cannot be set if `pr-required` is `false`. # (optional - default `1`) required-approvals = 1 # Which GitHub teams have access to push/merge to this branch. diff --git a/rust_team_data/src/v1.rs b/rust_team_data/src/v1.rs index d6cecfa01..e2826f3df 100644 --- a/rust_team_data/src/v1.rs +++ b/rust_team_data/src/v1.rs @@ -205,12 +205,21 @@ pub enum RepoPermission { Triage, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "snake_case")] +pub enum BranchProtectionMode { + PrRequired { + ci_checks: Vec, + required_approvals: u32, + }, + PrNotRequired, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct BranchProtection { pub pattern: String, - pub ci_checks: Vec, pub dismiss_stale_review: bool, - pub required_approvals: u32, + pub mode: BranchProtectionMode, pub allowed_merge_teams: Vec, } diff --git a/src/schema.rs b/src/schema.rs index c24b45acf..492933b5f 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -790,6 +790,8 @@ pub(crate) struct BranchProtection { pub dismiss_stale_review: bool, #[serde(default)] pub required_approvals: Option, + #[serde(default = "default_true")] + pub pr_required: bool, #[serde(default)] pub allowed_merge_teams: Vec, } diff --git a/src/static_api.rs b/src/static_api.rs index 28c3e4bef..e268172fc 100644 --- a/src/static_api.rs +++ b/src/static_api.rs @@ -4,6 +4,7 @@ use anyhow::{ensure, Context as _, Error}; use indexmap::IndexMap; use log::info; use rust_team_data::v1; +use rust_team_data::v1::BranchProtectionMode; use std::collections::HashMap; use std::path::Path; @@ -48,9 +49,15 @@ impl<'a> Generator<'a> { .iter() .map(|b| v1::BranchProtection { pattern: b.pattern.clone(), - ci_checks: b.ci_checks.clone(), dismiss_stale_review: b.dismiss_stale_review, - required_approvals: b.required_approvals.unwrap_or(1), + mode: if b.pr_required { + BranchProtectionMode::PrRequired { + ci_checks: b.ci_checks.clone(), + required_approvals: b.required_approvals.unwrap_or(1), + } + } else { + BranchProtectionMode::PrNotRequired + }, allowed_merge_teams: b.allowed_merge_teams.clone(), }) .collect(); diff --git a/src/validate.rs b/src/validate.rs index d14babee6..52ac57220 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -793,6 +793,25 @@ but that team does not seem to exist"#, } } + if !protection.pr_required { + // It does not make sense to use CI checks when a PR is not required, because with a + // CI check, it would not be possible to push into the branch without a PR anyway. + if !protection.ci_checks.is_empty() { + bail!( + r#"repo '{}' uses a branch protection for {} that does not require a PR, but has non-empty `ci-checks`"#, + repo.name, + protection.pattern, + ); + } + if protection.required_approvals.is_some() { + bail!( + r#"repo '{}' uses a branch protection for {} that does not require a PR, but sets the `required-approvals` attribute"#, + repo.name, + protection.pattern, + ); + } + } + if bors_used { if protection.required_approvals.is_some() { bail!( diff --git a/tests/static-api/_expected/v1/repos.json b/tests/static-api/_expected/v1/repos.json index 84b511a8d..0f6274491 100644 --- a/tests/static-api/_expected/v1/repos.json +++ b/tests/static-api/_expected/v1/repos.json @@ -16,11 +16,15 @@ "branch_protections": [ { "pattern": "master", - "ci_checks": [ - "CI" - ], "dismiss_stale_review": false, - "required_approvals": 1, + "mode": { + "pr_required": { + "ci_checks": [ + "CI" + ], + "required_approvals": 1 + } + }, "allowed_merge_teams": [] } ], @@ -43,11 +47,15 @@ "branch_protections": [ { "pattern": "master", - "ci_checks": [ - "CI" - ], "dismiss_stale_review": false, - "required_approvals": 1, + "mode": { + "pr_required": { + "ci_checks": [ + "CI" + ], + "required_approvals": 1 + } + }, "allowed_merge_teams": [ "foo" ] diff --git a/tests/static-api/_expected/v1/repos/archived_repo.json b/tests/static-api/_expected/v1/repos/archived_repo.json index 0519633b5..d5ce4510d 100644 --- a/tests/static-api/_expected/v1/repos/archived_repo.json +++ b/tests/static-api/_expected/v1/repos/archived_repo.json @@ -14,11 +14,15 @@ "branch_protections": [ { "pattern": "master", - "ci_checks": [ - "CI" - ], "dismiss_stale_review": false, - "required_approvals": 1, + "mode": { + "pr_required": { + "ci_checks": [ + "CI" + ], + "required_approvals": 1 + } + }, "allowed_merge_teams": [] } ], diff --git a/tests/static-api/_expected/v1/repos/some_repo.json b/tests/static-api/_expected/v1/repos/some_repo.json index 6b3b3158d..2bb557395 100644 --- a/tests/static-api/_expected/v1/repos/some_repo.json +++ b/tests/static-api/_expected/v1/repos/some_repo.json @@ -14,11 +14,15 @@ "branch_protections": [ { "pattern": "master", - "ci_checks": [ - "CI" - ], "dismiss_stale_review": false, - "required_approvals": 1, + "mode": { + "pr_required": { + "ci_checks": [ + "CI" + ], + "required_approvals": 1 + } + }, "allowed_merge_teams": [ "foo" ]