From 32b993b6ef32634735bff1f89a07285e84d4ca35 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 13 Jun 2026 11:22:25 -0700 Subject: [PATCH] docs(contributor): streamline create-pr verification --- .../nemoclaw-contributor-create-pr/SKILL.md | 48 ++++++++++++++----- .github/PULL_REQUEST_TEMPLATE.md | 5 +- AGENTS.md | 4 +- CONTRIBUTING.md | 23 +++++---- docs/AGENTS.md | 3 +- docs/CONTRIBUTING.md | 6 +-- 6 files changed, 61 insertions(+), 28 deletions(-) diff --git a/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md b/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md index 06a7e8ef35e..0a7f8fd82d1 100644 --- a/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md +++ b/.agents/skills/nemoclaw-contributor-create-pr/SKILL.md @@ -35,27 +35,47 @@ Before creating a PR, verify the branch. git status ``` -## Step 2: Run Pre-PR Checks +## Step 2: Choose Efficient Pre-PR Checks -Choose checks based on the files changed. +Do not rerun the whole local gate just to create a PR when Git hooks already supplied that evidence. +Use the checks that match the diff and the verification you already have. -For code changes, run both checks and confirm they pass before proceeding: +### Hook Evidence + +If the commits were created normally and the branch was pushed normally, count the installed hooks as verification: + +- `pre-commit` runs file fixers, formatters, linters, docs-to-skills dry-run validation, and changed-surface Vitest hooks. +- `commit-msg` runs commitlint. +- `pre-push` runs TypeScript build and type-check gates. + +If hooks were skipped with `--no-verify`, were not installed, failed, or you cannot tell whether they ran, run a manual diff-scoped fallback before creating the PR: ```bash -npx prek run --all-files -npm test +npx prek run --from-ref main --to-ref HEAD ``` -For doc-only changes, do not run the full test suite unless the docs change requires it. -Run the docs and hook checks instead: +Use `npx prek run --all-files` only when you need a whole-repository baseline, such as changing hook configuration, formatter configuration, generated-check scripts, or other repo-wide validation behavior. + +### Targeted Tests + +Run the smallest meaningful tests for changed behavior: + +- CLI or root `src/`, `bin/`, `scripts/`, or `test/` changes: `npx vitest run --project cli` or the directly affected test file. +- Plugin changes under `nemoclaw/src/`: `npx vitest run --project plugin` or the directly affected plugin test file. +- E2E support changes under `test/e2e-scenario/support-tests/`: `npx vitest run --project e2e-vitest-support`. +- Installer behavior changes: run the relevant installer integration project only when the local environment supports it. + +Reserve full `npm test` for broad runtime changes, test harness changes, or cases where targeted coverage is hard to justify. +Do not run the full test suite for doc-only changes unless the docs change code samples or generated behavior in a way that needs runtime validation. + +For doc-only changes, run the docs build before opening the PR: ```bash -npx prek run --all-files npm run docs ``` If a required check fails, fix the issue before creating the PR. -When preparing the PR body for a doc-only change, leave the `npm test` verification box unchecked unless you actually ran it. +When preparing the PR body, check only the verification boxes backed by hooks, manual commands, or CI evidence you actually have. ## Step 3: Push the Branch @@ -129,8 +149,9 @@ Use the exact template structure below. Fill in each section based on the diff ( ## Verification -- [ ] `npx prek run --all-files` passes -- [ ] `npm test` passes +- [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes +- [ ] Targeted tests pass for changed behavior +- [ ] Full `npm test` passes (broad runtime changes only) - [ ] Tests added or updated for new or changed behavior - [ ] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes @@ -151,7 +172,7 @@ Follow these rules when filling in the template: - **Related Issue:** Include `Fixes #NNN` or `Closes #NNN` if an issue exists. Remove the section entirely if there is no related issue. - **Changes:** Bullet list of key changes. Be specific — reference file names, commands, or behaviors that changed. - **Type of Change:** Check exactly one box. Use `[x]` for checked, `[ ]` for unchecked. -- **Verification:** Check only the boxes for steps you actually ran and confirmed passing. Do not check boxes for steps you skipped or did not verify. For doc-only changes, `npm test` is not required; leave it unchecked unless you ran it. +- **Verification:** Check only the boxes for steps you actually ran and confirmed passing, or for Git hooks that passed during normal commit and push. Do not check boxes for steps you skipped or did not verify. For doc-only changes, `npm test` is not required; leave it unchecked unless you ran it. - **DCO Sign-Off:** Replace `{name}` and `{email}` with values from `git config user.name` and `git config user.email`. ## Step 6: Create the PR @@ -198,7 +219,8 @@ Created PR [#NNN](https://github.com/NVIDIA/NemoClaw/pull/NNN) - **Do not invent your own PR body format.** Use the template from Step 5 exactly. - **Do not omit sections.** Even if a section is not applicable, keep it with the "Skip if..." comment. - **Do not check boxes for steps you did not run.** If you did not run `npm run docs`, leave that box unchecked. -- **Do not run the full test suite for doc-only changes by default.** Run docs and hook checks instead, and leave `npm test` unchecked unless you actually ran it. +- **Do not rerun hook-covered checks by default.** Normal commit and push hooks are valid verification. Use `npx prek run --from-ref main --to-ref HEAD` as the fallback when hooks were skipped, missing, or uncertain. +- **Do not run the full test suite for doc-only changes by default.** Run the docs build instead, and leave `npm test` unchecked unless you actually ran it. - **Do not forget the DCO sign-off.** CI will reject the PR without it. - **Do not forget `--assignee @me`.** Every PR must be assigned to its creator. - **Do not create PRs from main.** Always use a feature branch. diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 7a754447dd6..465e162b368 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -17,8 +17,9 @@ ## Verification -- [ ] `npx prek run --all-files` passes -- [ ] `npm test` passes +- [ ] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes +- [ ] Targeted tests pass for changed behavior +- [ ] Full `npm test` passes (broad runtime changes only) - [ ] Tests added or updated for new or changed behavior - [ ] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes diff --git a/AGENTS.md b/AGENTS.md index e5eaebc6643..1fc7d26750c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -195,7 +195,9 @@ All hooks managed by [prek](https://prek.j178.dev/) (installed via `npm install` ## PR Requirements - Create feature branch from `main` -- Run `make check` and `npm test` before submitting +- Let normal commit and push hooks provide hook verification before submitting +- Run targeted tests for changed behavior, and run `npm run docs` for doc changes +- Use `npx prek run --from-ref main --to-ref HEAD` if hooks were skipped or unavailable - Follow PR template (`.github/PULL_REQUEST_TEMPLATE.md`) - No secrets, API keys, or credentials committed - Limit open PRs to fewer than 10 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8feb761b549..a3c358b9652 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -138,27 +138,32 @@ All git hooks are managed by [prek](https://prek.j178.dev/), a fast, single-bina | **commit-msg** | commitlint (Conventional Commits) | | **pre-push** | TypeScript type check (`tsc --noEmit` for plugin, JS, and CLI) | -For a full manual check: `npx prek run --all-files`. For scoped runs: `npx prek run --from-ref --to-ref HEAD`. +For PR preparation, normal commit and push hooks are valid verification when they ran without `--no-verify`. +If hooks were skipped, missing, failed, or uncertain, use a scoped fallback: `npx prek run --from-ref --to-ref HEAD`. +Reserve `npx prek run --all-files` for whole-repository baselines, such as hook, formatter, generated-check, or repo-wide validation changes. For TypeScript changes under `src/`, `test/`, `scripts/`, `bin/`, or -`nemoclaw-blueprint/scripts/` (and for `tsconfig.cli.json` updates), also run -`npm run typecheck:cli` before opening a PR. CI runs this unconditionally, and the -pre-push hook runs it with `tsconfig.cli.json` before pushes. +`nemoclaw-blueprint/scripts/` (and for `tsconfig.cli.json` updates), the pre-push +hook runs `npm run typecheck:cli` before the branch is pushed. +CI runs this unconditionally. +If the pre-push hook was skipped or unavailable, run `npm run typecheck:cli` +manually before opening a PR. If you still have `core.hooksPath` set from an old Husky setup, Git will ignore `.git/hooks`. Run `git config --unset core.hooksPath` in this repo, then `npm install` so `prek install` (via `prepare`) can register the hooks. `make check` remains the primary documented linter entry point. For doc-only changes, you do not need to run the full test suite by default. -Run the docs and hook checks instead: +Commit and push normally so the hooks run, then run the docs build: ```bash -npx prek run --all-files npm run docs ``` Leave `npm test` unchecked in the PR verification checklist unless you actually ran it. -Run `npm test` when the change touches code, generated behavior, or anything that affects runtime behavior. +If hooks were skipped or unavailable, run `npx prek run --from-ref main --to-ref HEAD` before opening the PR. +For code changes, run targeted tests for the changed behavior. +Reserve full `npm test` for broad runtime changes, test harness changes, or cases where targeted coverage is hard to justify. ## Project Structure @@ -241,7 +246,9 @@ Follow these steps to submit a pull request. 1. Create a feature branch from `main`. 2. Make your changes with tests. -3. Run the relevant checks. For code changes, run `make check` and `npm test`. For doc-only changes, run `npx prek run --all-files` and `npm run docs`. +3. Run the relevant checks. + Let normal commit and push hooks provide hook verification, run targeted tests for changed behavior, and run `npm run docs` for doc changes. + If hooks were skipped or unavailable, run `npx prek run --from-ref main --to-ref HEAD`. 4. Open a PR. ### Commit Messages diff --git a/docs/AGENTS.md b/docs/AGENTS.md index b0e44281939..5f58307a32a 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -43,5 +43,6 @@ Treat `docs/` as the source of truth for published content and generated user sk - Run `npm run docs:sync-agent-variants` after editing shared variant source pages or navigation. - Run `npm run docs` before opening a PR for docs or Fern changes. -- For doc-only PRs, run `npx prek run --all-files` unless the user asks for a narrower draft. +- For doc-only PRs, rely on normal commit and push hooks when they ran. + If hooks were skipped or unavailable, run `npx prek run --from-ref main --to-ref HEAD`. - Leave `npm test` unchecked in the PR verification checklist unless you actually ran it. diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index fa4f4563f42..20b1989cefb 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -129,15 +129,15 @@ Do not convert route-style links to `.mdx` file links just to satisfy a local fi ## Doc-Only PR Verification Doc-only pull requests do not need the full test suite by default. -Before opening a doc-only PR, run: +Commit and push normally so the Git hooks run, then run: ```bash -npx prek run --all-files npm run docs ``` Leave `npm test` unchecked in the PR verification checklist unless you actually ran it. -Run the full tests only when the change also touches code, generated behavior, or runtime behavior. +If hooks were skipped or unavailable, run `npx prek run --from-ref main --to-ref HEAD` before opening the PR. +Run targeted tests only when the change also touches code, generated behavior, or runtime behavior. ## Writing Conventions