镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Clarify contributor and agent guidance from PR feedback - #8744

Merged
dmerand merged 1 commit into
mainfrom
donald/pr-review-guidance-20261002
Oct 5, 2026
Merged

dmerand merged 1 commit into
mainfrom
donald/pr-review-guidance-20261002

Conversation

@dmerand

@dmerand dmerand commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

PR feedback includes repeated corrections for resource cleanup, cache identity, and regression test inputs. These examples come from a review of available feedback from 2 April to 2 October 2026.

Some expectations are already documented but need clearer links. Other guidance needs more detail. Keep the added details in existing guides so contributors and agents have one place to find each rule.

WHAT is this pull request doing?

Keep the existing guide list in AGENTS.md and explain when to read each page. Clarify guidance for cache validity, resource cleanup, file IO, error recovery, diagnostics, performance, and regression tests.

This changes documentation and agent instructions, not the CLI implementation.

How to manually test your changes?

  1. Read the AGENTS.md guide list. Follow the links for CLI, CLI kit, and UI kit work.
  2. Review the added paragraphs in the existing guides. Check that resource ownership, failure behavior, and required test outcomes are clear.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Oct 2, 2026
@dmerand
dmerand requested a balanced review from Copilot October 2, 2026 19:53

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.

Copilot review overview

🟢 Approval recommended

The documentation-only changes consistently capture the cited feedback and use valid repository references.

Review effort: Balanced
Findings: None

What changed in this PR

Consolidates recurring PR feedback into contributor and agent guidance without changing CLI behavior.

Changes:

  • Clarifies cache identity, resource cleanup, I/O, diagnostics, recovery, and performance guidance.
  • Strengthens regression-testing and command-flag testing expectations.
  • Improves guide discovery and supported-tooling instructions.
File Description
AGENTS.md Adds task-oriented guide descriptions and clearer agent rules.
.agents/​automated-tasks/​performance.md Moves performance evidence from code comments to PRs.
.agents/​automated-tasks/​security.md Clarifies secure path-containment validation.
docs/​README.md Repairs plugin guidance links.
docs/​cli/​conventions.md Documents cache, resource, I/O, and observability conventions.
docs/​cli/​cross-os-compatibility.md Updates supported Node and PNPM setup.
docs/​cli/​debugging.md Adds diagnostic redaction guidance.
docs/​cli/​error_handling.md Clarifies narrow recovery and retry handling.
docs/​cli/​get-started.md References authoritative runtime versions.
docs/​cli/​performance.md Adds measurement and concurrency guidance.
docs/​cli/​testing-strategy.md Strengthens regression and cleanup testing guidance.
docs/​cli-kit/​command-guidelines.md Adds dependent-flag testing guidance.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@dmerand
dmerand marked this pull request as ready for review October 2, 2026 20:06
@dmerand
dmerand requested a review from a team as a code owner October 2, 2026 20:06
@dmerand
dmerand added this pull request to the merge queue Oct 5, 2026
Tests can be run with `pnpm test` for the Vitest suite, `pnpm test:watch` for watch mode, or `pnpm test:e2e` for the Playwright end-to-end suite. If you want to run a single unit test, pass the path to the file as argument:
- `pnpm test`: run the Vitest suite.
- `pnpm exec vitest`: run Vitest in watch mode.
- `pnpm test:e2e`: run the Playwright end-to-end suite.

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.

This is not enough, you need some env variables that are explained here: https://vault.shopify.io/teams/2238-DevTools/docs/dev-platform/cli/Testing#test-credentials

What about adding a link?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll follow up with a fix.

Merged via the queue into main with commit 0cf3104 Oct 5, 2026
79 of 82 checks passed
@dmerand
dmerand deleted the donald/pr-review-guidance-20261002 branch October 5, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants