[MAINT] Improve Import behaviour of `github_default_branch` by deiga · Pull Request #3216 · integrations/terraform-provider-github

deiga · 2026-02-21T20:01:37Z

Resolves #2613

Before the change?

Resource didn't use Context-aware functions
Resource didn't use tflog for logging
Missing tests for Update and Import functionality

After the change?

Changes to use Context-aware functions
Changes to use tflog for logging
Adds support for repository renaming, by add repository_id field and removing ForceNew from repository

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)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

Yes
No

github-actions · 2026-02-21T20:01: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

These provider review instructions are being used.

This PR updates the github_branch_default resource to improve import and rename behavior, modernize the resource implementation to SDKv2 context-aware CRUD, and align logging and tests with current provider patterns.

Changes:

Refactors github_branch_default to use CreateContext/ReadContext/UpdateContext/DeleteContext, returns diag.Diagnostics, and uses tflog.
Adds rename-safe repository handling via repository_id + CustomizeDiff: diffRepository, and updates docs/templates to document the new exported attribute.
Expands acceptance coverage to include update and import scenarios using statecheck/plancheck.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
`github/resource_github_branch_default.go`	Refactors resource to context-aware CRUD + `tflog`, adds `repository_id`, custom import handler, and rename-safe diffing.
`github/resource_github_branch_default_test.go`	Adds/updates acceptance tests using modern `statecheck`/`plancheck` patterns for create/update/import coverage.
`docs/resources/branch_default.md`	Documents exported `repository_id` attribute and clarifies import ID format.
`templates/resources/branch_default.md.tmpl`	Mirrors docs updates for generated documentation (`repository_id` + import wording).

stevehipwell

Just a quick pre-pass to look at the coding patterns for a re-work. The multi line tflog statements make reviewing harder; as does the type checking but I think we need to keep that (other than for meta in the resource functions).

stevehipwell

It looks like the code for the repository_id addition is missing (e.g. migration, import, update ID modification). I'd also suggest that the docs be updated to the new pattern as part of this change given the attributes are being modified.

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

deiga · 2026-06-05T19:24:25Z

It looks like the code for the repository_id addition is missing (e.g. migration, import, update ID modification). I'd also suggest that the docs be updated to the new pattern as part of this change given the attributes are being modified.

🤦 Doh!
Good catch, adding that now

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

stevehipwell · 2026-06-08T08:24:45Z

-			if _, _, err := client.Repositories.RenameBranch(ctx, owner, repoName, *repository.DefaultBranch, defaultBranch); err != nil {
-				return err
+			tflog.Debug(ctx, "Renaming branch to new default")
+			if _, _, err := client.Repositories.RenameBranch(ctx, owner, repoName, repository.GetDefaultBranch(), defaultBranch); err != nil {


Should we clear the etag here to make it explicit that we don't expect it to work on this code path (not that it ever works with non-static tokens)?

What do you mean? It's only used for the reading the repo

Yes, but if you rename the branch you're explicitly modifying the repo which means the repository read is stale and requires a new etag. By manually clearing it you're making this clear to anyone reading the code.

stevehipwell · 2026-06-08T08:28:30Z

+		if repository.GetDefaultBranch() != defaultBranch {
+			tflog.Debug(ctx, "Renaming branch to new default")
+			etag = resp.Header.Get("ETag")
+			if _, _, err := client.Repositories.RenameBranch(ctx, owner, repoName, repository.GetDefaultBranch(), defaultBranch); err != nil {


Same etag question as above.

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

stevehipwell

If you wait for #3476 to be merged you can reuse the repo ID migration logic.

stevehipwell · 2026-06-12T15:12:33Z

+func resourceGithubBranchDefaultImport(ctx context.Context, d *schema.ResourceData, m any) ([]*schema.ResourceData, error) {
+	repoName := d.Id()
+
+	if err := d.Set("repository", repoName); err != nil {
+		return nil, err
+	}
+
+	return []*schema.ResourceData{d}, nil


We need to fetch all of the required fields as part of the import (e.g. repo ID & branch).

github-actions Bot added the Type: Bug Something isn't working as documented label Feb 21, 2026

deiga added this to the v6.12.0 Release milestone Feb 22, 2026

deiga force-pushed the fix-default-branch-import branch from 0e63837 to a31fc9f Compare March 8, 2026 21:14

deiga requested a review from stevehipwell March 8, 2026 21:15

deiga modified the milestones: v6.12.0 Release, v6.13.0 Release Apr 14, 2026

deiga force-pushed the fix-default-branch-import branch from a31fc9f to cd4dd3d Compare May 11, 2026 02:08

deiga marked this pull request as draft May 14, 2026 21:10

deiga removed the request for review from stevehipwell May 17, 2026 13:22

deiga force-pushed the fix-default-branch-import branch from b44b2a4 to 687ec04 Compare June 3, 2026 17:16

deiga marked this pull request as ready for review June 3, 2026 17:16

deiga force-pushed the fix-default-branch-import branch from b417ac4 to 055d00c Compare June 3, 2026 18:25

deiga requested a review from Copilot June 3, 2026 18:26

Copilot started reviewing on behalf of deiga June 3, 2026 18:26 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

Comment thread github/resource_github_branch_default.go

Comment thread github/resource_github_branch_default.go

Comment thread github/resource_github_branch_default.go Outdated

Comment thread github/resource_github_branch_default.go

Comment thread github/resource_github_branch_default_test.go

deiga requested a review from stevehipwell June 3, 2026 19:50

stevehipwell requested changes Jun 4, 2026

View reviewed changes

Comment thread github/resource_github_branch_default.go Outdated

Comment thread github/resource_github_branch_default.go Outdated

Comment thread github/resource_github_branch_default.go Outdated

deiga force-pushed the fix-default-branch-import branch from 9d51b80 to 85640c2 Compare June 4, 2026 11:18

stevehipwell requested changes Jun 5, 2026

View reviewed changes

Comment thread github/resource_github_branch_default.go Outdated

Comment thread github/resource_github_branch_default.go Outdated

deiga added 10 commits June 5, 2026 21:51

Refactor to use Context-aware functions

8a3178d

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Sort CRUD functions

8106007

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Updates naming and error handling

8492313

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Adds test cases for Update and Import functionality

48528a1

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Refactor to use tflog

0ca131d

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Remove Read calls from Update and Create

7b201e3

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Update tests to use ConfigStateChecks

c665e57

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Adds repository_id field

85f7924

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Adds custom Import function

27e5911

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Updates docs

8de8716

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

deiga added 8 commits June 5, 2026 21:51

gofmt

0b285e2

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Make forcetypeassert linter happy

391b0d5

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Rename branch only if it's different from current default branch

08d8c27

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Fix documentation error

6933c46

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Add acc tests to verify Destroy behaviour

fedf817

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Address review comments

326af96

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Use errors.AsType pattern

330a466

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Address review comments

f88a893

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

deiga added 2 commits June 5, 2026 22:42

Add state migration and tests

f59df2b

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Update docs template to be mostly generated

4137ac7

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

deiga force-pushed the fix-default-branch-import branch from efdac3c to 4137ac7 Compare June 5, 2026 20:06

deiga requested review from Copilot and stevehipwell June 5, 2026 20:07

Copilot started reviewing on behalf of deiga June 5, 2026 20:08 View session

Copilot AI reviewed Jun 5, 2026

View reviewed changes

Comment thread github/resource_github_branch_default.go

deiga added 2 commits June 6, 2026 08:34

Ensure that ID gets updated if reponame changes

ba987f0

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Remove typeasserts from schema fields

0a739c7

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

stevehipwell requested changes Jun 8, 2026

View reviewed changes

Address review comments

6b9e94a

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

deiga requested a review from stevehipwell June 12, 2026 06:36

stevehipwell requested changes Jun 12, 2026

View reviewed changes

Conversation

deiga commented Feb 21, 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 Feb 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

deiga commented Jun 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevehipwell Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

deiga Jun 11, 2026

Choose a reason for hiding this comment

Uh oh!

stevehipwell Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

stevehipwell Jun 8, 2026

Choose a reason for hiding this comment

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

stevehipwell Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

deiga commented Feb 21, 2026 •

edited

Loading