fix: Correct repository environment diff function logic#3469
fix: Correct repository environment diff function logic#3469stevehipwell wants to merge 1 commit into
Conversation
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
|
👋 Hi, and thank you for this contribution! This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can. You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions. 🤖 This is an automated message. |
There was a problem hiding this comment.
issue: Could we use more descriptive variable names for this? The code is highly nested and it's really hard to parse what each single char variable is supposed to be.
I don't care how idiomatic for golang this is supposed to be, it's poorly readable code
There was a problem hiding this comment.
Pull request overview
These provider review instructions are being used. This PR fixes a CustomizeDiff panic during upgrades/migrations of github_repository_environment when the prior state has an empty/nil reviewers element, and it updates acceptance coverage around adding/removing reviewers.
Changes:
- Harden
resourceGithubRepositoryEnvironmentDiffto avoid nil/type assertion panics whenreviewersis present but not shaped as expected. - Adjust the v0 migration schema for
reviewersto better match historical state shape during upgrades. - Extend acceptance tests to assert expected
reviewerslist size across updates.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
github/resource_github_repository_environment.go |
Makes reviewers counting in CustomizeDiff more defensive to prevent panics during plan/upgrade. |
github/resource_github_repository_environment_test.go |
Adds acceptance assertions for reviewers list size across update scenarios. |
github/resource_github_repository_environment_migration.go |
Updates the v0 schema used for state upgrading to align with older reviewers state layout. |
| if v, ok := d.GetOk("reviewers"); ok { | ||
| count := 0 | ||
| o := v.([]any)[0] | ||
| if t, ok := o.(map[string]any)["teams"]; ok { | ||
| count += t.(*schema.Set).Len() | ||
| } | ||
| if c, ok := v.([]any); ok && len(c) > 0 { | ||
| if o, ok := c[0].(map[string]any); ok { | ||
| count := 0 | ||
|
|
Resolves #3455
Before the change?
github_repository_environmentwith no reviewers couldn't be migratedAfter the change?
github_repository_environmentcan be successfully migratedPull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!