Skip to content

ci: add review criteria for the shared review skill - #7

Merged
ezekiel-alexrod merged 1 commit into
mainfrom
ci/add-review-criteria
Jul 28, 2026
Merged

ci: add review criteria for the shared review skill#7
ezekiel-alexrod merged 1 commit into
mainfrom
ci/add-review-criteria

Conversation

@ezekiel-alexrod

Copy link
Copy Markdown
Collaborator

The reusable review workflow runs the /review-pr skill from the Scality agent
hub. That skill reads its criteria from .claude/REVIEW.md, and when the file is
missing it says so in its own summary:

💡 No review instructions found. Add a .github/copilot-instructions.md or .claude/REVIEW.md file to improve quality of review.

This adds the file rather than forking the skill into the repo. The skill keeps
its own posting mechanism and its upstream updates, and this file stays a plain
description of what matters here:

  • the contract between the two halves — a change to the cache layout has to land
    in the agent, in the package and in agent/DESIGN.md at once
  • archive entries are attacker-controlled input, so extraction safety gets its
    own row
  • crash safety: a killed agent must never leave a cache that looks complete
  • CRD, label and installed-path compatibility, since consumers pin to them
  • EL8/EL9 parity for the packaging half
  • what not to flag: everything golangci-lint, shellcheck, rpmlint and gofmt
    already own

The criteria mention agent/, which lands with #4. Merging this first only means
the agent rows apply a few days early.

Side effect worth noting: this PR is also the first real exercise of the app-key
pass-through merged in #6. It does not touch review.yaml, so the workflow
validation gate passes and the review should actually run — which #6 itself could
not demonstrate.

The reusable review workflow runs the `/review-pr` skill from the Scality
agent hub, which reads its criteria from `.claude/REVIEW.md`. Without that
file the review is generic and the skill says so in its own summary.

Write the criteria instead of forking the skill into the repo: the skill
keeps its posting mechanism and its updates, and this file stays a plain
description of what matters here — the contract between the agent and the
package, hostile archive input, crash safety, CRD and label compatibility,
EL8/EL9 parity — plus the list of findings the linters already own.
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.

1 participant