Skip to content

feat(contrib): add PR review and module scaffold skills#131

Open
jeanscherf wants to merge 4 commits into
mainfrom
feat/sdk-contribution-skills
Open

feat(contrib): add PR review and module scaffold skills#131
jeanscherf wants to merge 4 commits into
mainfrom
feat/sdk-contribution-skills

Conversation

@jeanscherf
Copy link
Copy Markdown
Member

@jeanscherf jeanscherf commented May 20, 2026 •
edited
Loading

Description

Adds two Claude Code skills to `.claude/skills/` for SDK contributors. Skills are invocable via Claude Code CLI (or any compatible agentic tool: Cline, Copilot, etc.) from the repo root.

`/review-pr [PR#]`

Reviews a PR against 23 criteria across 6 sections:

  • A: Process & Compliance: PR template, conventional commits, issue link, AI disclosure
  • B: Security: credentials, internal URLs, sensitive data in code and PR body
  • C: Code Quality: CI checks, version bump, type hints, naming, imports, dependencies, proto freshness
  • D: API & Design: future-proofing, public API hygiene, breaking changes, pagination, telemetry
  • E: Tests & Documentation: test coverage, docstring quality, module structure

Outputs a structured table report with ✅/⚠️/❌/➖ per criterion, a blocking issues summary, and an optional post via `gh pr review`.

Line citations are accurate because Phase 2 fetches each changed file at the PR head commit SHA via `gh api`, so all `file:line` references in the report come from `Read` output on the actual files rather than from diff hunk arithmetic.

Accepts a plain PR number (defaults to SAP/cloud-sdk-python), a full GitHub URL, or an `owner/repo#number` reference, so it works for PRs in forks as well as in the upstream repo.

`/scaffold-module`

Generates the full standard module layout for a new BTP service integration:

  • 10 files: `init.py`, `client.py`, `config.py`, `exceptions.py`, `_models.py`, `py.typed`, `user-guide.md`, plus test stubs
  • Telemetry wiring: inserts `Module.` into `core/telemetry/module.py` in alphabetical order
  • Self-review phase: runs the full local gate (`ruff check`, `ruff format --check`, `ty check`, `pytest`) and fixes issues before reporting
  • Prints a checklist of remaining manual steps

Related Issue

Closes AFSDK-3286

Type of Change

  • New feature (non-breaking change which adds functionality)

How to Test

  1. From the repo root, run `/review-pr 128` to verify a structured 23-criterion report is produced with accurate `file:line` citations.
  2. Run `/review-pr 93` to verify the skill handles a large new-module PR across all 23 criteria.
  3. Provide a fork PR URL (e.g. `github.com//cloud-sdk-python/pull/1`) to verify the skill fetches files from the fork repo.
  4. Run `/scaffold-module` with test inputs (e.g. module=`test_svc`, service=`Test Service`, short=`TS`) to verify 10 files are generated, the full quality gate passes, and the generated files can then be deleted.

Checklist

@jeanscherf
feat(contrib): add PR review and module scaffold skills
2577360 
Adds two Claude Code skills to .claude/skills/ for SDK contributors:

- review-pr: reviews a PR against 23 criteria across 6 sections (process,
  security, code quality, API design, tests, documentation). Runs from the
  repo root via gh CLI. Outputs a structured table report with verdict and
  optional gh pr review posting.

- scaffold-module: generates the full standard module layout (10 files +
  telemetry wiring) for a new BTP service integration. Includes a self-review
  phase that runs ruff and fixes issues before reporting.

Both skills are compatible with Claude Code, Cline, Copilot, and other
agentic tools via the tools/compatibility frontmatter fields.
@jeanscherf jeanscherf requested a review from a team as a code owner May 20, 2026 17:18
@jeanscherf
fix(review-pr): fetch files at PR head ref for accurate line citations
6ea3087 
Replace diff-offset line counting with explicit file fetches. Phase 2
now downloads each changed file to /tmp/pr<number>/<path> at the PR
head ref; Phase 3 reads those files with the Read tool for exact line
numbers instead of deriving positions from unified diff hunk arithmetic.
tiagoek
tiagoek reviewed May 21, 2026
View reviewed changes
Comment thread .claude/skills/scaffold-module/SKILL.md Outdated
tiagoek
tiagoek reviewed May 21, 2026
View reviewed changes
Comment thread .claude/skills/scaffold-module/SKILL.md Outdated
tiagoek
tiagoek reviewed May 21, 2026
View reviewed changes
Comment thread .claude/skills/scaffold-module/SKILL.md Outdated
jeanscherf added 2 commits May 21, 2026 13:05
@jeanscherf
fix(review-pr): support fork PRs and full GitHub URLs
60e759d 
Phase 1 now parses REPO and NUMBER from three input forms: full GitHub
URL, owner/repo#number, or plain number (defaults to SAP/cloud-sdk-python).
All gh commands and the file-fetch API call use the resolved REPO so the
skill works for PRs in forks, not just PRs in the upstream repo.

Also switch file-fetch ref from branch name to head commit SHA to avoid
ambiguity when fork and upstream share the same branch name.
@jeanscherf
fix(scaffold-module): address review feedback from tiagoek
82fca05 
- Remove SAP-internal Artifactory URL from user-guide.md template;
  replace with plain `uv add sap-cloud-sdk`
- Remove incorrect token_url derivation from service URL; BTP binding
  schemas vary and the UAA URL must be read from the binding, not
  guessed from the service URL
- Expand Phase 5 self-review to run the full local gate: ruff check,
  ruff format --check, ty check, and pytest (not just ruff check)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tiagoek tiagoek tiagoek approved these changes

@betinacosta betinacosta betinacosta approved these changes

@ArthurTonial ArthurTonial ArthurTonial approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jeanscherf @tiagoek @betinacosta @ArthurTonial