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

data source directory_role_templates #1152

Merged

Conversation

MattGarnerAWR
Copy link
Contributor

@MattGarnerAWR MattGarnerAWR commented Jul 24, 2023

My 1st attempt at a PR to add a new data source for directory role templates :)

@MattGarnerAWR MattGarnerAWR marked this pull request as ready for review July 24, 2023 10:24
@MattGarnerAWR MattGarnerAWR changed the title directory role templates data source directory_role_templates Jul 24, 2023
Copy link
Member

@manicminer manicminer left a comment

Choose a reason for hiding this comment

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

Hi @MattGarnerAWR, thank you for this PR! This is looking great, and I do not have any changes to suggest beyond what the tests have flagged - just a couple questions to clarify before we look to merge this.

@MattGarnerAWR
Copy link
Contributor Author

MattGarnerAWR commented Jul 24, 2023

Hi @MattGarnerAWR, thank you for this PR! This is looking great, and I do not have any changes to suggest beyond what the tests have flagged - just a couple questions to clarify before we look to merge this.

Thanks @manicminer , I have resolved all of them and hopefully the right package will mean the tests run this time :)

Copy link
Member

@manicminer manicminer left a comment

Choose a reason for hiding this comment

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

Thanks @MattGarnerAWR, just a couple minor tweakd which I've made below but otherwise LGTM 👍

@manicminer
Copy link
Member

@MattGarnerAWR Just a couple things to note:

  • New resources / data sources need to be registered, typically there's a registration.go file in each package for this
  • Whenever we dereference a pointer, we need to first check that it's non-nil or it may cause a crash at runtime

I fixed these up in the PR as I didn't spot them earlier on, hope you don't mind. Thanks again for the contribution!

@manicminer manicminer added this to the v2.41.0 milestone Jul 24, 2023
@manicminer manicminer merged commit f159113 into hashicorp:main Jul 24, 2023
7 checks passed
manicminer added a commit that referenced this pull request Jul 24, 2023
@MattGarnerAWR
Copy link
Contributor Author

@MattGarnerAWR Just a couple things to note:

  • New resources / data sources need to be registered, typically there's a registration.go file in each package for this
  • Whenever we dereference a pointer, we need to first check that it's non-nil or it may cause a crash at runtime

I fixed these up in the PR as I didn't spot them earlier on, hope you don't mind. Thanks again for the contribution!

Thanks for your help!

@github-actions
Copy link

This functionality has been released in v2.41.0 of the Terraform 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 following the template. Thank you!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants