Configure Copilot Code Review for the provider repo by robert-crandall · Pull Request #3425 · integrations/terraform-provider-github

robert-crandall · 2026-05-13T19:06:33Z

This PR adds Copilot instructions to this repo.

Before the change?

The repo had no .github/copilot-instructions.md and no path-scoped .github/instructions/*.md files, so Copilot Code Review (CCR) ran with no project-specific guidance. That meant a lot of churn on PRs: low-value nits, suggestions based on older Go toolchains, and missing context about provider-specific conventions (notably the intentional "no read-after-write" pattern, see [MAINT]: Remove read call from create/update #2892).

After the change?

Added .github/copilot-instructions.md with the repo-wide review policy:
- Go 1.24 toolchain anchor so CCR doesn't suggest pre-1.22 loop-variable shadowing, rewrite range over int, or flag generics/min/max/clear/slices/maps as unknown.
- Severity and nit policy: only HIGH and MEDIUM findings are reported. No LOW, no style/formatting/naming/comment-wording feedback (those belong to linters and human reviewers).
- Carve-outs for things that always matter even if they look like nits: Sensitive: true on secret-bearing attributes, schema Description, ValidateFunc/ValidateDiagFunc on bounded inputs, examples for new resources, docs for schema changes, import ID format.
- Repo-specific rule: do not flag missing read-after-write in create/update; that is intentional ([MAINT]: Remove read call from create/update #2892).
Added four path-scoped instruction files under .github/instructions/:
- schema-and-state.instructions.md — applies to github/**/*.go; covers schema/state compatibility, CRUD behavior, API safety.
- tests.instructions.md — applies to github/**/*_test.go; coverage expectations and test structure.
- examples.instructions.md — applies to examples/**; standard module layout, no nested provider blocks, sensitive variable handling.
- docs.instructions.md — applies to website/**; keeping docs in sync with schema, documenting import IDs and permission scopes.

I think this should noticeably cut the contributor friction CCR has been generating, especially around Go-version false positives and stylistic nits. If something turns out to be too restrictive (or not restrictive enough), the instruction files are easy to iterate on.

Pull request checklist

Schema migrations have been created if needed (example)
Tests for the changes have been added (for bug fixes / features)
Docs have been reviewed and added / updated if needed (for bug fixes / features)

Docs/tests checkboxes left unchecked: this PR only touches .github/ config for CCR. No provider code, no schema, no user-facing docs are affected.

Does this introduce a breaking change?

Yes
No

github-actions · 2026-05-13T19:06:45Z

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

Copilot

Pull request overview

Adds repository-wide and path-scoped Copilot Code Review (CCR) instruction files to align automated review feedback with this provider’s conventions (notably Go 1.24 toolchain assumptions and the intentional “no read-after-write” pattern from #2892), reducing low-value churn on PRs.

Changes:

Added repo-wide CCR guidance in .github/copilot-instructions.md (toolchain anchors, severity policy, provider-specific review priorities).
Added path-scoped CCR instruction files for provider Go code, tests, examples, and website docs under .github/instructions/.

Show a summary per file

File	Description
.github/copilot-instructions.md	Defines repo-wide CCR policy (Go/tooling context, severity rules, provider-specific review checklist).
.github/instructions/schema-and-state.instructions.md	Path-scoped guidance for `github/*/.go` focusing on schema/state compatibility, CRUD behavior, and API safety.
.github/instructions/tests.instructions.md	Path-scoped guidance for `github/*/_test.go` covering expectations for test updates and acceptance testing.
.github/instructions/examples.instructions.md	Path-scoped guidance for `examples/**` enforcing example structure, provider config rules, and sensitive handling.
.github/instructions/docs.instructions.md	Path-scoped guidance for `website/**` ensuring docs stay in sync with schema, imports, and permissions.

Copilot's findings

Tip

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

Files reviewed: 5/5 changed files
Comments generated: 0

deiga

This is looking really good!

- Correct Go version anchor: 1.24 -> 1.26 (matches go.mod). - Drop ValidateFunc everywhere; ValidateDiagFunc only (ValidateFunc is deprecated and not allowed in this repo). - Rewrite docs.instructions.md to target templates/** instead of the no-longer-existing website/**. Docs under docs/** are auto-generated; edits go in templates/, examples/, or resource Description fields. - Replace remaining website/ references in copilot-instructions.md with docs/ and templates/. - Add repository-rename convention to schema-and-state.instructions.md: attribute must be 'repository' (not ForceNew), plus computed 'repository_id', plus CustomizeDiff: diffRepository. - Add Delete 404 handling: treat as success, not error. - Require *Context CRUD variants and diag.Diagnostics on all new resources. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

robert-crandall · 2026-05-13T20:35:29Z

Thanks for the review @deiga! Addressed all 11 comments in 96bacb6:

.github/copilot-instructions.md

Go version anchor 1.24 -> 1.26 (it really is 1.26 in go.mod; I was looking at an older checkout).
Dropped ValidateFunc everywhere (deprecated, not allowed in this repo). ValidateDiagFunc only.
website/ references -> docs/ and templates/.

.github/instructions/schema-and-state.instructions.md

Added a new "Repository as a Required Argument" section covering the rename-safe pattern: attribute named repository (not ForceNew), computed repository_id, CustomizeDiff: diffRepository. Pointed at customdiff.All(...) for resources that already chain a CustomizeDiff.
Delete now explicitly treats 404 as success, separate from the Read-404-removes-from-state rule.
All CRUD funcs (including importer StateContext) must use the *Context variants and return diag.Diagnostics.
Dropped ValidateFunc; ValidateDiagFunc only.

.github/instructions/docs.instructions.md

Rewrote against reality: website/** doesn't exist, docs/** is auto-generated and is reverted by a CI workflow if touched manually. applyTo now targets templates/**. The file explains that edits belong in templates/, examples/, or resource Description fields, and flags any manual docs/** edit as HIGH.

Let me know if any of the framing on the repository-rename pattern needs tweaking; that one was new to me and I leaned on util_diff.go plus the existing CustomizeDiff: diffRepository callsites for the convention.

deiga · 2026-05-13T21:02:29Z

The new changes cover pretty much everything I noticed! Thanks @robert-crandall

I just remembered that it could be useful to include mention that we now use https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing and it's conventions should be followed for any changes or additions in tests.

Check should be migrated to (most often) ConfigStateChecks
Custom pointer structures for testing value changes should be replaced with ValueComparers https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests/value-comparers

Add a 'terraform-plugin-testing Conventions' section to .github/instructions/tests.instructions.md covering: - Prefer ConfigStateChecks over the legacy Check / ComposeTestCheckFunc pattern. - Use ValueComparers (compare.ValuesSame / compare.ValuesDiffer) for cross-step value comparisons instead of custom pointer structs. - Don't flag legacy patterns in unmodified tests; only in new or substantially modified ones. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

robert-crandall · 2026-05-13T21:51:17Z

The new changes cover pretty much everything I noticed! Thanks @robert-crandall

I just remembered that it could be useful to include mention that we now use https://pkg.go.dev/github.com/hashicorp/terraform-plugin-testing and it's conventions should be followed for any changes or additions in tests.

Check should be migrated to (most often) ConfigStateChecks

Custom pointer structures for testing value changes should be replaced with ValueComparers https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests/value-comparers

Good call! Added a terraform-plugin-testing Conventions section to .github/instructions/tests.instructions.md in b415e8f:

stevehipwell · 2026-05-14T14:14:31Z

@robert-crandall how about https://awesome-copilot.github.com/instructions/#file=instructions%2Fgo.instructions.md for the Go code? I have some customizations to suggest op top of that but it's a good starting point for generic Go code.

Incorporates the upstream Go instructions from https://github.com/github/awesome-copilot/blob/main/instructions/go.instructions.md as a path-scoped instructions file (applyTo: **/*.go,**/go.mod,**/go.sum). A preamble subordinates the Go guidance to the repo's existing severity policy (HIGH/MEDIUM only, no LOW or nits) and notes the Go-version anchors in copilot-instructions.md plus the no-read-after-write exception, so CCR doesn't suddenly start surfacing style nits or fight provider-specific conventions. The main copilot-instructions.md now lists every path-scoped file so the set is discoverable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

robert-crandall · 2026-05-15T18:20:31Z

@robert-crandall how about https://awesome-copilot.github.com/instructions/#file=instructions%2Fgo.instructions.md for the Go code? I have some customizations to suggest op top of that but it's a good starting point for generic Go code.

Good idea! Done in 73cda9b

stevehipwell

Just a couple of standard changes to the go based instructions.

Co-authored-by: Steve Hipwell <steve.hipwell@gmail.com>

stevehipwell · 2026-05-19T11:24:27Z

+
+# Docs and Templates Review
+
+Provider docs under `docs/**` are **auto-generated**. Do not edit files


I think it's probably worth including some context around the usage of hashicorp/terraform-plugin-docs and the folder layout it expects in templates and examples (note our examples aren't in the correct format to be automatically used). It's also worth noting that if you're not adding additional content you can use the default template.

We also ought to call out the make commands that can update the docs locally.

stevehipwell · 2026-05-19T11:26:36Z

+
+## Keep Docs in Sync with Schema
+
+- Any schema change in `github/` (new attribute, renamed attribute,


Should we make it explicit that any changes are expected to be made to the schema so we're fixing forward?

stevehipwell · 2026-05-19T11:30:02Z

+- Resources that support `terraform import` must document the import ID
+  format with at least one example command in the relevant template.


This should just be a check that the docs include a section for import as this should be auto generated by the template.

stevehipwell · 2026-05-19T11:30:32Z

+- For any GitHub API call that requires a specific token scope or app
+  permission, the template should call this out so users can configure
+  their authentication correctly.


Do we want this to link directly to the REST API page?

stevehipwell · 2026-05-19T11:31:14Z

+- Inline example HCL should reflect current argument names and be
+  syntactically valid.
+- Sensitive attributes should not appear with real-looking secrets, even
+  as examples.


A part of the automation these are checked with terraform fmt, so this is the baseline.

stevehipwell · 2026-05-19T11:37:50Z

+applyTo: "templates/**"
+---
+
+# Docs and Templates Review


Should we not link these instructions to .github/copilot-instructions.md?

stevehipwell · 2026-05-19T11:38:25Z

+applyTo: "**/*.go,**/go.mod,**/go.sum"
+---
+
+# Go Development Instructions


Should we not link these instructions to .github/copilot-instructions.md?

stevehipwell · 2026-05-19T11:38:51Z

+# Provider Source Review (Schema, State, API)
+
+These rules apply to all provider Go source files under `github/`. Combine
+with the repo-wide checklist in `.github/copilot-instructions.md`.


Do we also need to link this to the Go instructions?

stevehipwell · 2026-05-19T11:40:43Z

+## API Safety and Performance
+
+- Flag new N+1 access patterns over GitHub APIs.
+- Verify pagination is handled (`ListOptions` / `NextPage` loops) on any


We should be using the iterator pattern for pagination.

stevehipwell · 2026-05-19T11:41:05Z

+applyTo: "github/**/*_test.go"
+---
+
+# Provider Test Review


Do we also need to link this to the Go instructions?

robert-crandall added 6 commits March 12, 2026 14:48

CCR

6d5bac1

Move skill around

be1616f

Prevent read after update

20ca971

Make review compatible with CCR

03e857f

Add nit policy

f2fb579

Pin go version for feedback

8cf2fd5

robert-crandall requested a review from Copilot May 13, 2026 19:10

Merge branch 'main' into robert-crandall/copilot/open-pr-request

35842df

robert-crandall marked this pull request as ready for review May 13, 2026 19:11

Copilot started reviewing on behalf of robert-crandall May 13, 2026 19:11 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

deiga requested changes May 13, 2026

View reviewed changes

robert-crandall requested a review from deiga May 15, 2026 18:20

Merge branch 'main' into robert-crandall/copilot/open-pr-request

0189be6

stevehipwell requested changes May 18, 2026

View reviewed changes

Apply suggestions from code review

fc287cd

Co-authored-by: Steve Hipwell <steve.hipwell@gmail.com>

robert-crandall requested a review from stevehipwell May 18, 2026 22:07

stevehipwell requested changes May 19, 2026

View reviewed changes


		# Docs and Templates Review

		Provider docs under `docs/` are auto-generated**. Do not edit files


		## Keep Docs in Sync with Schema

		- Any schema change in `github/` (new attribute, renamed attribute,

		- Resources that support `terraform import` must document the import ID
		format with at least one example command in the relevant template.

Conversation

robert-crandall commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before the change?

After the change?

Pull request checklist

Does this introduce a breaking change?

Uh oh!

github-actions Bot commented May 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

deiga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

robert-crandall commented May 13, 2026

Uh oh!

deiga commented May 13, 2026

Uh oh!

robert-crandall commented May 13, 2026

Uh oh!

stevehipwell commented May 14, 2026

Uh oh!

robert-crandall commented May 15, 2026

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

robert-crandall commented May 13, 2026 •

edited

Loading