Skip to content

Commit

Permalink
Add branch protection mode
Browse files Browse the repository at this point in the history
  • Loading branch information
Kobzol committed Mar 19, 2024
1 parent 76ad8d6 commit 81651f2
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 20 deletions.
5 changes: 5 additions & 0 deletions docs/toml-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 11 additions & 2 deletions rust_team_data/src/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
required_approvals: u32,
},
PrNotRequired,
}

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
pub struct BranchProtection {
pub pattern: String,
pub ci_checks: Vec<String>,
pub dismiss_stale_review: bool,
pub required_approvals: u32,
pub mode: BranchProtectionMode,
pub allowed_merge_teams: Vec<String>,
}

Expand Down
2 changes: 2 additions & 0 deletions src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -790,6 +790,8 @@ pub(crate) struct BranchProtection {
pub dismiss_stale_review: bool,
#[serde(default)]
pub required_approvals: Option<u32>,
#[serde(default = "default_true")]
pub pr_required: bool,
#[serde(default)]
pub allowed_merge_teams: Vec<String>,
}
11 changes: 9 additions & 2 deletions src/static_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down
19 changes: 19 additions & 0 deletions src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
24 changes: 16 additions & 8 deletions tests/static-api/_expected/v1/repos.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": []
}
],
Expand All @@ -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"
]
Expand Down
12 changes: 8 additions & 4 deletions tests/static-api/_expected/v1/repos/archived_repo.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": []
}
],
Expand Down
12 changes: 8 additions & 4 deletions tests/static-api/_expected/v1/repos/some_repo.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
Expand Down

0 comments on commit 81651f2

Please sign in to comment.