Skip to content

chore: enforce peer-reviewed PRs on main, release/*, point/*#621

Open
yurii-vasyliev wants to merge 7 commits into
mainfrom
chore/branch-protection-codeowners
Open

chore: enforce peer-reviewed PRs on main, release/*, point/*#621
yurii-vasyliev wants to merge 7 commits into
mainfrom
chore/branch-protection-codeowners

Conversation

@yurii-vasyliev

@yurii-vasyliev yurii-vasyliev commented May 14, 2026

Copy link
Copy Markdown
Contributor

Why

Landscape's quality management commits us to peer-reviewed code changes — the requirement comes from ISO 9001 §8.3.4 (design & development controls) and §7.5 (documented information for change control), plus ISO/IEC 27001 A.8.32 (change management) and A.8.28 (secure coding). Until now landscape-ui had no enforced PR / review gate at the GitHub layer, so the control existed only as convention.

The branch ruleset that supplies the gate (id 16386358) is already deployed on canonical/landscape-ui and managed in the GitHub UI — the UI now carries its own version history, which is sufficient for the audit trail. This PR adds the two repo-level pieces the ruleset needs to actually work on every protected branch.

What this PR does

File Purpose
.github/CODEOWNERS Routes every path to @canonical/landscape-ui, which lets the branch ruleset require review from a Code Owner.
.github/workflows/lint.yml, run-tests-and-tics.yml, changeset-check.yml Extends pull_request.branches to include release/* and point/* so the required status checks actually fire on PRs into those protected branches. Without this, PRs targeting release/point would be blocked forever waiting on checks that never report.

An earlier revision of this PR also committed protected-branches.json + a README under .github/rulesets/ as the ISO 9001 §7.5 artifact, but the manual sync between committed JSON and the live ruleset turned into a constant source of drift (every Copilot review thread on this PR traces back to that). GitHub's built-in ruleset version history now covers the audit-trail need without the friction; if a version-controlled snapshot is ever required, it can be re-exported with gh api .../rulesets/16386358 | jq ....

What the live ruleset enforces (FYI, managed in the UI, not part of this PR)

Applies to main, release/*, and point/*:

  • PR required before any change reaches a protected branch — no direct pushes.
  • ≥ 1 approval from @canonical/landscape-ui (Code Owner review required, stale approvals dismissed on new pushes, last-push approval required).
  • Required status checks (strict — branch must be up to date), all pinned to integration_id: 15368 (GitHub Actions):
    • ESLint, Prettier, Stylelint
    • unit-tests, e2e-tests (saas), e2e-tests (self-hosted)
    • verify
  • Linear history, no force-push, no deletion.
  • Allowed merge methods: squash + rebase (no merge commits).
  • Copilot code review runs automatically on every push to a PR targeting a protected branch.
  • Bypass list: empty — controls apply to admins too.

changeset-release/* branches are unaffected because they don't match release/* or point/* (different prefix segments), so the Changesets bot keeps working without an explicit exclusion.

Test plan

  • CI green on this PR.
  • After merge, Insights → Code owners shows @canonical/landscape-ui covering *.
  • Open a throwaway PR against main and confirm it can't merge without an approval from a @canonical/landscape-ui member.
  • Open a throwaway PR against release/26.04 and confirm ESLint / Prettier / Stylelint / unit-tests / e2e-tests (saas) / e2e-tests (self-hosted) / verify all run and block merge until they pass.
  • Open a throwaway PR against a point/* branch and confirm the same checks run.
  • Try to force-push to release/1.23.1 and confirm it's rejected.
  • Confirm the next Changesets-bot PR from changeset-release/* still merges normally.

Follow-ups (out of scope)

  • If release/* and point/* warrant a stricter gate (e.g. 2 approvals), layer a second ruleset targeting just those patterns rather than tightening the shared one.
  • If a versioned snapshot of the live ruleset becomes useful for audit hygiene, re-export it as a periodic dump rather than a continuously-edited source of truth.

yurii-vasyliev and others added 2 commits May 14, 2026 10:16
Add CODEOWNERS so reviews from @canonical/landscape-ui can be required
by GitHub's branch ruleset, and check in the ruleset definition itself
as version-controlled documented information (ISO 9001 §7.5).

The JSON file does not apply itself; see .github/rulesets/README.md
for the gh CLI commands a repo admin runs to push it to GitHub.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Match the live state exported from canonical/landscape-ui so the file is
a faithful ISO 9001 §7.5 record. Changes:

- Name follows the GitHub label ("Protected release branches").
- Adds copilot_code_review rule (review_on_push: true).
- Drops the unused changeset-release/** exclude — those branches don't
  match release/* or point/* anyway.
- Status-check contexts use the fully-qualified
  "<workflow name> / <job id>" form GitHub displays in the Checks tab.
- required_review_thread_resolution: false to match live.
- README documents the deployed ruleset id and a diff command to verify
  the committed JSON keeps matching reality.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds version-controlled GitHub governance configuration so protected branches can require peer-reviewed, code-owner-approved changes with documented ruleset management.

Changes:

  • Adds a repository-wide CODEOWNERS rule for @canonical/landscape-ui.
  • Adds a protected-branches ruleset JSON for main, release/*, and point/*.
  • Documents how to apply, inspect, and diff the ruleset.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
.github/CODEOWNERS Defines the default code owner for all paths.
.github/rulesets/protected-branches.json Adds the branch ruleset enforcing reviews, signed commits, linear history, and required checks.
.github/rulesets/README.md Documents ruleset operations and required status-check mappings.

Comment thread .github/rulesets/protected-branches.json Outdated
Comment thread .github/rulesets/protected-branches.json Outdated
Comment thread .github/rulesets/README.md Outdated
Comment thread .github/rulesets/README.md Outdated
Comment thread .github/rulesets/protected-branches.json Outdated
Comment thread .github/rulesets/README.md Outdated
@yurii-vasyliev yurii-vasyliev requested a review from a team May 14, 2026 08:35
ethanashaw
ethanashaw previously approved these changes May 14, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

.github/rulesets/protected-branches.json:50

  • These required test check contexts do not match the current PR test workflow name in .github/workflows/run-tests-and-tics.yml (Run tests and TICS report). Because required status checks are matched by their emitted context, the configured Tests + TICS on PRs / ... checks will not be produced unless the workflow name or these contexts are updated consistently.
          { "context": "Tests + TICS on PRs / unit-tests" },
          { "context": "Tests + TICS on PRs / e2e-tests" },

Comment thread .github/rulesets/protected-branches.json Outdated
Comment thread .github/rulesets/README.md Outdated
@github-actions

Copy link
Copy Markdown
Contributor

TICS Quality Gate

✔️ Passed

No changed files applicable for TICS analysis quality gating.

Run-tests-and-TICS-report / tics-report / TICS report

Address Copilot review feedback on PR #621:

- protected-branches.json: replace fully-qualified status-check contexts
  (e.g. "Lint & format / eslint") with the bare check_run.name GitHub
  Actions actually posts (ESLint, unit-tests, e2e-tests (saas), etc.),
  pinned to integration_id 15368. The previous values could never match
  any emitted check and would have left protected PRs unmergeable.
- protected-branches.json: restore branch scope to main, release/*, and
  point/* — the JSON now matches the deployed ruleset (16386358) exactly,
  so the diff command in the README produces a clean output.
- README.md: rewrite the "Required status checks" guidance to describe
  how contexts actually match (bare name + integration_id), refresh the
  context-to-job mapping table, and fix the diff command (._links, not
  .links; also strips bypass_actors so [] vs null doesn't noise the diff).
- lint.yml, run-tests-and-tics.yml, changeset-check.yml: extend
  pull_request.branches to include release/* and point/* so the required
  checks actually report on PRs into those protected branches. Without
  this, the ruleset blocks merges forever waiting on checks that never
  run. changeset-check.yml narrows release/** to release/* to match the
  ruleset; no existing release/point branch uses nested segments.
Copilot AI review requested due to automatic review settings May 20, 2026 13:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread .github/rulesets/protected-branches.json Outdated
Comment thread .github/rulesets/protected-branches.json Outdated
The committed JSON never auto-applied — every change needed a manual PUT
to the rulesets API, which left committed source and live config drifting
apart and was the root cause of every Copilot review comment on this PR.
GitHub's built-in ruleset version history in the UI now covers the audit
trail this folder was created for, so the manual-sync ceremony is no
longer worth the friction.

If a version-controlled record is needed later, the live ruleset can be
exported back with:

    gh api /repos/canonical/landscape-ui/rulesets/16386358 \
      | jq 'del(.id, .source, .source_type, .created_at, .updated_at,
                .node_id, .current_user_can_bypass, ._links)' \
      > .github/rulesets/protected-branches.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants