I’ve taken a look at the provider here and would like to say great work so far! I do have feedback outlined below that I’d like to see addressed before we move on to the next steps. I’m opening this issues as a sort of checklist for tracking items and discussion. The review was done on the develop branch at git commit 9bb6592
[ ] The provider repository name should only contain lower cases characters. So the repository name should be changed from terraform-provider-vThunder to terraform-provider-vthunder
[ ] A makefile should be added and named GNUmakefile which is standard across all Terraform providers, feel free to copy the NULL provider makefile and will just have to update the variable on line 4 to PKG_NAME=vthunder
[ ] Along with the makefile there are four bash scripts kept in a scripts/ directory that are used in different processes of testing and release, these can all be copied from the NULL provider
[ ] All Terraform providers have now switched to using Go modules for managing and vendoring dependancies for the provider but in addition to that, we ask that you check-in all the required dependancies into a vendor/ directory. This is so that our nightly testing environment does ping GitHub for ever dependancies in all 100+ TF providers.
[ ] Any .tf files that you have for examples for provider usage should be kept under an ‘examples/’ directory
[ ] All official Terraform providers use a Mozilla Public License 2.0 within each repo, can you add this LICENSE file to this repository.
[ ] You will need to build website documentation in a website/ directory that will be hosted on Terraform.io for each resource and data source, these are all written in Markdown with a ruby layout file for the sidebar formatting. Check out the Github provider website docs as a good example.
[ ] All providers in the terraform-providers/ GitHub org have a TravisCI check run on each PR that is opened. Can you please add a .travis.yaml file to make the standard configuration
[ ] Each Terraform provider must have a CHANGELOG that is updated during reach release. Since the changeling is updated by a bot during the release it must be in a certain format. With the AWS provider changelog as an example, the version of the next release should be tagged as “1.0.0 (Unreleased)” and will then be updated with a date by the release-bot.
[ ] Any commented out and unused code should be removed from the provider.
[ ] Attributes should be read and set on Read so that differences between the configuration and the actual state can be caught. Any non-scalar values (TypeList, TypeSet, TypeMap) should have errors checked when setting. The reasoning for this is that the provider should never panic.
[ ] If a resource cannot be found(404) in the READ function, we should do a d.SetId("") to remove it from the statefile. You also wouldn't need to return an error then. Terraform will just treat it as a resource that never existed and try and create it.
[ ] The Ethernet resource Read, Update, and Delete functions have not been implemented yet. If they are not being used, the functions should be removed.
[ ] The CREATE method of any Resource should at least return the results of it’s own READ method. In certain cases it may need to return the results of the UPDATE method, depending on if the CREATE method is capable of setting all possible configuration values. If for example the API call to create Resource only allows basic creation, the remaining configuration is applied via update, then CREATE should return UPDATE, and UPDATE should return READ. AWS Security Group Resource is an example of this; the CREATE only allows creating a Security Group with a name and description, and we must make other API calls to apply the rules. Thus, CREATE returns the results of UPDATE.
For a number of reasons I was not able to run the entire acceptance test suite. Once the makefile, scripts and dependancies are add, then we can start running the acceptance tests.
Let me know if you have any questions about the feedback.
I’ve taken a look at the provider here and would like to say great work so far! I do have feedback outlined below that I’d like to see addressed before we move on to the next steps. I’m opening this issues as a sort of checklist for tracking items and discussion. The review was done on the develop branch at git commit
9bb6592[ ] The provider repository name should only contain lower cases characters. So the repository name should be changed from
terraform-provider-vThundertoterraform-provider-vthunder[ ] A makefile should be added and named GNUmakefile which is standard across all Terraform providers, feel free to copy the NULL provider makefile and will just have to update the variable on line 4 to PKG_NAME=vthunder
[ ] Along with the makefile there are four bash scripts kept in a
scripts/directory that are used in different processes of testing and release, these can all be copied from the NULL provider[ ] All Terraform providers have now switched to using Go modules for managing and vendoring dependancies for the provider but in addition to that, we ask that you check-in all the required dependancies into a
vendor/directory. This is so that our nightly testing environment does ping GitHub for ever dependancies in all 100+ TF providers.[ ] Any .tf files that you have for examples for provider usage should be kept under an ‘examples/’ directory
[ ] All official Terraform providers use a Mozilla Public License 2.0 within each repo, can you add this LICENSE file to this repository.
[ ] You will need to build website documentation in a website/ directory that will be hosted on Terraform.io for each resource and data source, these are all written in Markdown with a ruby layout file for the sidebar formatting. Check out the Github provider website docs as a good example.
[ ] All providers in the terraform-providers/ GitHub org have a TravisCI check run on each PR that is opened. Can you please add a .travis.yaml file to make the standard configuration
[ ] There is a standard README.md for of the official provider, see Null provider README.md
[ ] Each Terraform provider must have a CHANGELOG that is updated during reach release. Since the changeling is updated by a bot during the release it must be in a certain format. With the AWS provider changelog as an example, the version of the next release should be tagged as “1.0.0 (Unreleased)” and will then be updated with a date by the release-bot.
[ ] Any commented out and unused code should be removed from the provider.
[ ] Attributes should be read and set on Read so that differences between the configuration and the actual state can be caught. Any non-scalar values (TypeList, TypeSet, TypeMap) should have errors checked when setting. The reasoning for this is that the provider should never
panic.[ ] If a resource cannot be found(404) in the READ function, we should do a d.SetId("") to remove it from the statefile. You also wouldn't need to return an error then. Terraform will just treat it as a resource that never existed and try and create it.
[ ] The Ethernet resource Read, Update, and Delete functions have not been implemented yet. If they are not being used, the functions should be removed.
[ ] The CREATE method of any Resource should at least return the results of it’s own READ method. In certain cases it may need to return the results of the UPDATE method, depending on if the CREATE method is capable of setting all possible configuration values. If for example the API call to create Resource only allows basic creation, the remaining configuration is applied via update, then CREATE should return UPDATE, and UPDATE should return READ. AWS Security Group Resource is an example of this; the CREATE only allows creating a Security Group with a name and description, and we must make other API calls to apply the rules. Thus, CREATE returns the results of UPDATE.
For a number of reasons I was not able to run the entire acceptance test suite. Once the makefile, scripts and dependancies are add, then we can start running the acceptance tests.
Let me know if you have any questions about the feedback.