Skip to content

TPT-3925: Use omitzero for ACLP alerts to support empty lists#928

Open
yec-akamai wants to merge 8 commits intoproj/aclp-linode-alertsfrom
fix-update-alerts-empty
Open

TPT-3925: Use omitzero for ACLP alerts to support empty lists#928
yec-akamai wants to merge 8 commits intoproj/aclp-linode-alertsfrom
fix-update-alerts-empty

Conversation

@yec-akamai
Copy link
Copy Markdown
Contributor

@yec-akamai yec-akamai commented Apr 6, 2026

📝 Description

Use omitzero for alerts to make sure it explicitly pass an empty list instead of null.

Address terraform test issue: linode/terraform-provider-linode#2259

✔️ How to Test

make test-unit

@yec-akamai yec-akamai requested review from a team as code owners April 6, 2026 17:07
@yec-akamai yec-akamai requested review from Copilot, mawilk90 and mgwoj and removed request for a team April 6, 2026 17:07
@yec-akamai yec-akamai added the bugfix for any bug fixes in the changelog. label Apr 6, 2026
@yec-akamai yec-akamai changed the title Fix ACLP alerts to use pointer TPT-3925: Fix ACLP alerts to use pointer Apr 6, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Linode instance “ACLP alerts” structures to use pointer-to-slice fields so callers can explicitly send empty alert lists ([]) instead of having the field omitted (nil) during JSON marshaling, addressing downstream Terraform test expectations.

Changes:

  • Change InstanceAlert and InstanceACLPAlertsOptions alert list fields from []int to *[]int.
  • Update unit tests to handle pointer alert lists when reading and constructing request options.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.

File Description
instances.go Switches alert list fields to *[]int to distinguish omitted vs explicitly empty arrays in JSON.
test/unit/instance_test.go Adjusts tests to dereference alert list pointers and updates request option construction accordingly.

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

@yec-akamai yec-akamai force-pushed the fix-update-alerts-empty branch from d651c58 to af4eaeb Compare April 6, 2026 19:17
@yec-akamai yec-akamai requested a review from zliang-akamai April 6, 2026 19:50
Copy link
Copy Markdown
Contributor

@mgwoj mgwoj left a comment

Choose a reason for hiding this comment

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

PR title does not match the final changes. Also the changes in the code are not aligned with the changes in the tests.

@yec-akamai yec-akamai changed the title TPT-3925: Fix ACLP alerts to use pointer TPT-3925: Use omitzero for ACLP alerts to support empty lists Apr 7, 2026
@yec-akamai
Copy link
Copy Markdown
Contributor Author

PR title does not match the final changes. Also the changes in the code are not aligned with the changes in the tests.

Updated the PR title. There is not much to change in the test case since we avoid to make breaking changes. Already verfied the omitzero works as expected across terraform.

@yec-akamai yec-akamai requested a review from mgwoj April 7, 2026 20:29
@mgwoj
Copy link
Copy Markdown
Contributor

mgwoj commented Apr 8, 2026

Which tests is showing that this change with omitzero works?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix for any bug fixes in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants