Add Copilot PR review instructions for modern Go 1.26 patterns#7316
Add Copilot PR review instructions for modern Go 1.26 patterns#7316kristenwomack wants to merge 1 commit intoAzure:mainfrom
Conversation
The Copilot PR reviewer flagged 5 false positives on PR Azure#7223 because it didn't know about Go 1.26 features (e.g. new(expr) for pointer literals) and incorrectly flagged missing imports that existed outside the diff context. These patterns are documented in cli/azd/AGENTS.md but the PR reviewer reads .github/instructions/*.instructions.md files, not AGENTS.md. This adds a focused Go-specific instruction file that covers: - new(expr) pointer literal syntax (Go 1.26) - Other modern Go patterns that should not be flagged - Guidance to check full file context, not just diff hunks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds Go-specific Copilot PR review instructions so automated reviews stop flagging Go 1.26+ language/features and avoid “missing import/symbol” false-positives when the diff doesn’t show full file context.
Changes:
- Introduces
.github/instructions/go.instructions.mdscoped to**/*.go. - Documents accepted modern Go patterns (e.g.,
new(expr), integerrange, test helpers). - Instructs the reviewer to validate imports/symbols against unchanged file sections before reporting issues.
| # Modern Go (1.26+) — PR Review Guidelines | ||
|
|
||
| This project uses **Go 1.26** (`cli/azd/go.mod`). Do not flag modern Go 1.26 | ||
| features as errors. | ||
|
|
||
| ## `new(expr)` creates typed pointers from values | ||
|
|
||
| `new(false)`, `new(true)`, `new(0)`, `new("s")` are **valid Go 1.26**. They |
There was a problem hiding this comment.
This hard-codes a specific Go version in reviewer instructions. To reduce maintenance churn (and avoid stale guidance if the toolchain changes), consider wording this to follow the go directive in cli/azd/go.mod (e.g., “This project targets the Go version declared in cli/azd/go.mod” / “Go 1.26+”), rather than asserting a fixed version in the instructions text.
| # Modern Go (1.26+) — PR Review Guidelines | |
| This project uses **Go 1.26** (`cli/azd/go.mod`). Do not flag modern Go 1.26 | |
| features as errors. | |
| ## `new(expr)` creates typed pointers from values | |
| `new(false)`, `new(true)`, `new(0)`, `new("s")` are **valid Go 1.26**. They | |
| # Modern Go — PR Review Guidelines | |
| This project targets the Go version declared in `cli/azd/go.mod`. Do not flag | |
| language features supported by that version as errors. | |
| ## `new(expr)` creates typed pointers from values | |
| `new(false)`, `new(true)`, `new(0)`, `new("s")` are valid in the target Go version. They |
|
|
||
| ## Other modern patterns to accept (not flag) | ||
|
|
||
| - `errors.AsType[*T](err)` — generic error unwrapping (replaces `var e *T; errors.As(err, &e)`) |
There was a problem hiding this comment.
errors.AsType is not a commonly recognized stdlib API name; if this is project-specific (or from a third-party package), the instructions should say which package defines it (or fully qualify it) so the reviewer doesn’t mistakenly ignore legitimate typos like errors.AsType vs errors.As usage.
| - `errors.AsType[*T](err)` — generic error unwrapping (replaces `var e *T; errors.As(err, &e)`) | |
| - `errors.AsType[*T](err)` — project-specific (non-stdlib) generic error unwrapping helper (replaces `var e *T; errors.As(err, &e)`) |
| - `for i := range n` — range over integers | ||
| - `t.Context()` — test context (replaces `context.Background()` in tests) | ||
| - `t.Chdir(dir)` — test directory change (replaces `os.Chdir` + deferred restore) | ||
| - `wg.Go(func() { ... })` — WaitGroup shorthand (replaces `wg.Add(1); go func() { defer wg.Done(); ... }()`) |
There was a problem hiding this comment.
wg.Go(...) is not a method on sync.WaitGroup in typical Go usage; documenting it as “WaitGroup shorthand” may cause the reviewer to miss a real issue when wg is a plain sync.WaitGroup. If this refers to a specific type used in this repo (e.g., errgroup.Group, a custom wrapper, or another concurrency helper), clarify which type/package wg is expected to be.
| - `wg.Go(func() { ... })` — WaitGroup shorthand (replaces `wg.Add(1); go func() { defer wg.Done(); ... }()`) | |
| - `g.Go(func() { ... })` — helper on concurrency groups (for example `errgroup.Group`) that wraps `wg.Add(1); go func() { defer wg.Done(); ... }()` |
|
@kristenwomack I think once #7301 merges this becomes redundant since it already adds copilot-instructions.md pointing to AGENTS.md which has the Go 1.26 patterns. Can we close this one after #7301 lands? |
This PR (#7316) is specific to Copilot's PR review process. The docs change in #7301 adds instructions to the docs. I'm open to the how we get this done, so please do make suggested changes, but we should have docs and also customize the copilot PR config. The intension of this PR: add .github/instructions/go.instructions.md — a path-scoped instruction file with applyTo: "**/*.go". In #7301, . PR #7301 is fantastic, and this PR is intended to make Copilot actually read the relevant pages during Go file reviews. Reference docs:
|
Summary
Fixes #7315
The Copilot PR reviewer flagged 5 false positives on #7223 because it didn't recognize Go 1.26 features (
new(expr)pointer literals) and incorrectly flagged imports that existed outside the diff context.The modern Go patterns are documented in
cli/azd/AGENTS.md, but the PR reviewer doesn't read that file — it reads.github/instructions/*.instructions.md.Changes
.github/instructions/go.instructions.md— New instruction file scoped to**/*.gothat tells the Copilot PR reviewer about:new(expr)pointer literal syntax (Go 1.26)errors.AsType,for i := range n,t.Context(),wg.Go(), etc.)