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

resource/gitlab_project: Fix passing false to API for explicitly set optional attributes #1152

Merged
merged 2 commits into from Jun 29, 2022

Conversation

timofurrer
Copy link
Member

I think at one point we have to decide on a consistent strategy about how and when to set optional attributes for the API options ... I have a few ideas and I'll write them up in an issue.

Closes: #1151

@timofurrer timofurrer self-assigned this Jun 27, 2022
@timofurrer timofurrer added this to the v3.16.0 milestone Jun 27, 2022
@github-actions github-actions bot added provider resource Adds or modifies a resource size/S labels Jun 27, 2022
Copy link
Collaborator

@PatrickRice-KSC PatrickRice-KSC left a comment

Choose a reason for hiding this comment

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

@timofurrer - So I ran a quick test, and I verified that this did fix the issue (which logically it would), but it feels like we should have at least one test with some of the "false default" values running to make sure the fix works. Thoughts?

@timofurrer
Copy link
Member Author

@PatrickRice-KSC absolutely. I've just added a test case, but didn't use use_custom_template, and the attributes required when importing from an URL to avoid a huge test setup. Since those are anyways false by default on upstream I think we are good here.

@timofurrer
Copy link
Member Author

@PatrickRice-KSC note that I've also added the fixes for the group resource as described in #1154.

@PatrickRice-KSC
Copy link
Collaborator

PatrickRice-KSC commented Jun 29, 2022

I've just added a test case, but didn't use use_custom_template, and the attributes required when importing from an URL to avoid a huge test setup. Since those are anyways false by default on upstream I think we are good here.

Makes complete sense, that'd be a ton of setup!

note that I've also added the fixes for the group resource as described in https://github.com/gitlabhq/terraform-provider-gitlab/issues/1154.

Excellent, nice to knock out a couple issues with one PR!

@timofurrer - Looking good, I'll go ahead and merge it! 🚀

@PatrickRice-KSC PatrickRice-KSC merged commit e183e8a into gitlabhq:main Jun 29, 2022
@github-actions
Copy link

github-actions bot commented Jul 7, 2022

This functionality has been released in v3.16.0 of the Terraform GitLab Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue. Thank you!

@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
provider resource Adds or modifies a resource size/M tests
2 participants