Skip to content

Add Copilot PR review instructions for modern Go 1.26 patterns #7316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kristenwomack wants to merge 1 commit into
base: main
Choose a base branch
from
+28 −0
Open

Add Copilot PR review instructions for modern Go 1.26 patterns #7316

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/instructions/go.instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
applyTo:
- "**/*.go"
---
# 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
Comment on lines +4 to +11
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
# 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

Copilot uses AI. Check for mistakes.
create a pointer to the given value. This replaces helper functions like
`to.Ptr(val)`. Do NOT suggest `boolPtr()` or `&localVar` replacements.

## Other modern patterns to accept (not flag)

- `errors.AsType[*T](err)` — generic error unwrapping (replaces `var e *T; errors.As(err, &e)`)
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
- `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)`)

Copilot uses AI. Check for mistakes.
- `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(); ... }()`)
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
- `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(); ... }()`

Copilot uses AI. Check for mistakes.
- `min()`, `max()`, `clear()` — built-in functions

## Review the full file, not just the diff

Before flagging missing imports or undefined references, verify the symbol isn't
already defined in unchanged portions of the file. The diff context may not show
all existing imports or declarations.
Loading