Skip to content

Adopt the shared Claude review skill and modernize the review workflow #6194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bert-e merged 2 commits into from
Jun 19, 2026
+45 −211
Merged

Adopt the shared Claude review skill and modernize the review workflow #6194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45108ee
ci: adopt shared review skill, move criteria to copilot-instructions
delthas Jun 18, 2026
c17953d
ci: modernize review workflow to latest standard
delthas Jun 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 0 additions & 188 deletions .claude/skills/review-pr/SKILL.md
View file
Open in desktop

This file was deleted.

20 changes: 20 additions & 0 deletions .github/copilot-instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!-- markdownlint-disable MD013 -->

# Code review instructions

Repo context lives in [CLAUDE.md](../CLAUDE.md) — read it first.

When reviewing a PR, analyze the changes against these criteria:

| Area | What to check |
| --------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Async error handling | uncaught promise rejections, missing error callbacks, swallowed errors in streams. Double callbacks in try/catch blocks (callback called in try then again in catch). |
| Async/await migration | when code is migrated from callbacks to async/await, verify: no leftover `callback` or `next` params, no mixed callback + promise patterns, proper try/catch around awaited calls, errors are re-thrown or handled (not silently swallowed). Watch for the anti-pattern: `try { cb(); } catch(err) { cb(err); }` where an exception after the first `cb()` triggers a second call. |
| S3/API contract | breaking changes to request/response formats, new error codes, header handling, missing XML response fields. |
| Dependency pinning | git-based deps (arsenal, vaultclient, bucketclient, werelogs, utapi, scubaclient) must pin to a tag, not a branch. |
| Logging | proper use of werelogs logger, no `console.log` in production code, log levels match severity. |
| Stream handling | backpressure, proper cleanup on error (`.destroy()`), no leaked file descriptors, missing error event handlers. |
| Config changes | backward compatibility with existing env vars and `config.json`, default values. |
| Security | command injection, header injection, XML external entity attacks, path traversal, SSRF in multi-backend requests. |
| Breaking changes | changes to public S3 API behavior, metadata schema changes, env var renames without backward compat. |
| Test quality | no `.only()` tests (eslint enforces this), assertions match the behavior being tested, `require()`/`import` at top of file (never inside `describe` or functions). |
48 changes: 25 additions & 23 deletions .github/workflows/review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,29 +1,31 @@
name: Code Review

Check warning on line 1 in .github/workflows/review.yml

View workflow job for this annotation

GitHub Actions / lint 

1:1 [document-start] missing document start "---"

Check warning on line 1 in .github/workflows/review.yml

View workflow job for this annotation

GitHub Actions / lint 

1:1 [document-start] missing document start "---"
# yamllint disable rule:line-length
run-name: "Code Review for #${{ github.event.pull_request.number || inputs.pr_number }}${{ github.event.pull_request.title && format(' : {0}', github.event.pull_request.title) }}"
# yamllint enable rule:line-length

on:
pull_request:
types: [opened, synchronize, labeled, unlabeled]
pull_request_target:
types: [opened, synchronize]
pull_request:
types: [opened, synchronize, labeled, unlabeled]
pull_request_target:
types: [opened, synchronize]
workflow_dispatch:
inputs:
pr_number:
description: Pull Request number to review
required: true

jobs:
review:
if: github.event_name == 'pull_request' && github.actor != 'dependabot[bot]'
uses: scality/workflows/.github/workflows/claude-code-review.yml@v2
secrets:
GCP_WORKLOAD_IDENTITY_PROVIDER: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }}
GCP_SERVICE_ACCOUNT: ${{ secrets.GCP_SERVICE_ACCOUNT }}
ANTHROPIC_VERTEX_PROJECT_ID: ${{ secrets.ANTHROPIC_VERTEX_PROJECT_ID }}
CLOUD_ML_REGION: ${{ secrets.CLOUD_ML_REGION }}
review:
if: github.event_name != 'pull_request_target' && github.actor != 'dependabot[bot]'
uses: scality/workflows/.github/workflows/claude-code-review.yml@v2
with:
allowed-tools: >-
${{ github.event_name == 'workflow_dispatch' && '"Bash(gh api repos/*/contents)"' || '' }}
secrets: inherit

review-dependency-bump:
if: github.event_name == 'pull_request_target' && github.actor == 'dependabot[bot]'
uses: scality/workflows/.github/workflows/claude-code-dependency-review.yml@v2
with:
ACTIONS_APP_ID: ${{ vars.ACTIONS_APP_ID }}
secrets:
GCP_WORKLOAD_IDENTITY_PROVIDER: ${{ secrets.GCP_WORKLOAD_IDENTITY_PROVIDER }}
GCP_SERVICE_ACCOUNT: ${{ secrets.GCP_SERVICE_ACCOUNT }}
ANTHROPIC_VERTEX_PROJECT_ID: ${{ secrets.ANTHROPIC_VERTEX_PROJECT_ID }}
CLOUD_ML_REGION: ${{ secrets.CLOUD_ML_REGION }}
ACTIONS_APP_PRIVATE_KEY: ${{ secrets.ACTIONS_APP_PRIVATE_KEY }}
review-dependency-bump:
if: github.event_name == 'pull_request_target' && github.actor == 'dependabot[bot]'
uses: scality/workflows/.github/workflows/claude-code-dependency-review.yml@v2
with:
ACTIONS_APP_ID: ${{ vars.ACTIONS_APP_ID }}
secrets: inherit
Loading