Skip to content
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.

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

add omitempty to json tag for struct fields that are optional #314

Merged
merged 2 commits into from Feb 18, 2022

Conversation

uturunku1
Copy link
Collaborator

@uturunku1 uturunku1 commented Feb 16, 2022

Description

I audited the repo looking for options struct fields to update for omitempty json tag:

The fields that are not required should have the tag because if their values are null then the fields will get omitted from the encoding.
The fields that are required should not have the tag.

This PR is bringing those two types of changes.

I have added omitempty to
Two url filter tags that are optional: https://www.terraform.io/cloud-docs/api-docs/admin/users#query-parameters
One shh key attribute said to be optional: https://www.terraform.io/cloud-docs/api-docs/oauth-tokens#request-body

And I have removed omitempty from three fields that are required. For those fields, I didn't need to look in the documentation to find out if they are optional or not. Right there, in the same go file, I can see that we are running a validation check (options.valid()) to verify that those fields are not empty values.

There will be a follow up PR where I'll be adding a few more options.valid() for ListOptions that have required fields so we can match the other ones that already have this validation.

Testing plan

External links

Output from tests (HashiCorp employees only)

$ TFE_ADDRESS="https://example" TFE_TOKEN="example" TF_ACC="1" go test ./... -v -tags=integration -run TestFunctionsAffectedByChange

...

@uturunku1 uturunku1 changed the title add omitempty to json attribute options struct field that are optional add omitempty to json tag for struct fields that are optional Feb 17, 2022
@uturunku1 uturunku1 marked this pull request as ready for review February 18, 2022 15:54
@uturunku1 uturunku1 requested a review from a team as a code owner February 18, 2022 15:54
@brandonc
Copy link
Collaborator

Any reason why we can't target main on this? Seems like a minor bug fix

Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

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

I think the Include fields on run_task and task_stages can also be omitempty. I missed them in my review of that endpoint

@uturunku1 uturunku1 merged commit 7945e7c into releases-1.0.x Feb 18, 2022
@uturunku1 uturunku1 deleted the update-omitempty branch February 18, 2022 18:28
sebasslash pushed a commit that referenced this pull request Feb 22, 2022
* add omitempty to json attribute options struct field that are optional

* remove omitempty from required fields
sebasslash pushed a commit that referenced this pull request Feb 22, 2022
* add omitempty to json attribute options struct field that are optional

* remove omitempty from required fields
sebasslash pushed a commit that referenced this pull request Feb 23, 2022
* add omitempty to json attribute options struct field that are optional

* remove omitempty from required fields
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants