Skip to content

chore: refine Copilot PR interface review prompt [no-ci]#52

Merged
lukeocodes merged 1 commit intomainfrom
chore/refine-copilot-pr-review-prompt
May 6, 2026
Merged

chore: refine Copilot PR interface review prompt [no-ci]#52
lukeocodes merged 1 commit intomainfrom
chore/refine-copilot-pr-review-prompt

Conversation

@lukeocodes
Copy link
Copy Markdown
Member

Refines .github/prompts/pr-interface-review.md based on Copilot review feedback raised during the original rollout:

  1. Step 1 / Step 5 consistency — Step 1 said "ignore tests except deleted" but Step 5 also requires reviewing added tests that prove compat shims. Broadened Step 1 to cover added, deleted, and modified-to-remove tests. (raised on deepgram/deepgram-go-sdk#329)

  2. Tier 6 broadened — original wording only mentioned same-named local symbols as collision vectors. Now also covers glob/namespace imports (Python from X import *, Rust use X::*, Go dot-imports, TypeScript import * as destructuring). (raised on deepgram/deepgram-js-sdk#495)

  3. Fence language — the template fenced block now declares markdown to satisfy markdownlint MD040. (raised on deepgram/deepgram-go-sdk#329)

Canonical source: deepgram/dx-stack. Rollout context: docs/copilot-prompts.md.

[no-ci] — docs-only refinement to a previously-merged file.

Copilot AI review requested due to automatic review settings May 6, 2026 13:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refines the shared Copilot PR interface/contract review prompt to improve consistency and correctness when reviewing public-surface changes across SDKs.

Changes:

  • Aligns Step 1 guidance about tests with Step 5 by requiring noting added/deleted/modified-to-remove compat-related tests.
  • Broadens Tier 6 “name collision” guidance to include glob/namespace-style import patterns as collision vectors.
  • Adds an explicit fenced-code language (markdown) to satisfy markdownlint MD040.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

| 🔍 **4** | **Type tightening / silent risk** | A type was narrowed (e.g. `str` → enum), a default removed, a docstring contract changed, or behavior subtly shifted. Compiles fine; may surprise users. |
| ➕ **5** | **Purely additive** | New optional field, new optional parameter with default, new method, new exported type. No existing caller can break. |
| 🆕 **6** | **Brand-new public type** | Entirely new type/class/symbol with no predecessor. Only breaks callers who happened to define a same-named local symbol. |
| 🆕 **6** | **Brand-new public type** | Entirely new type/class/symbol with no predecessor. Only breaks callers via name collision — same-named local symbol, glob imports (`from X import *`, `use X::*`), Go dot-imports, or TypeScript `import * as` destructuring. |
@lukeocodes lukeocodes merged commit de2dd4b into main May 6, 2026
12 checks passed
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.

3 participants