rubrikinc/terraform-provider-rubrik

Terraform Provider Development Program - Review

cgriggs01 opened this issue · 3 comments

cgriggs01 commented

Hi Team,

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. If it works better for you, this changes can be added to a separate branch if any changes conflict with your current build process. The review was done on the develop branch at git commit a93dce2

  • The name the of this repository must be changed to match the standard terraform naming convention —> terraform-provider-rubrik

  • All of the official Terraform providers host their website documentation on Terrafom.io/docs/providers/index.html/. We’ve recently published some guidelines on provider documentationCheck out the Github provider website docs as a good example.

  • The resources you’ve build for AWS and Azure CloudOn & CloudOut do not have corresponding acceptance test. Do you plan to have tests for these resources or is there a reason to not have tests for these?

  • You are currently using a MIT license but all official Terraform provider use a Mozilla Public License 2.0 within each repo, can you add this LICENSE file to this repository.

  • There are a few scripts required during the build and release process of the provider. Could you copy over the script you don’t have yet from the Github provider scripts and remove the unnecessary docscheck.sh and websitefmtcheck.sh

  • There are a number of file that will not be necessary and should be removed from the repository. CODE_OF_CONDUCT, CONTRIBUTING, TESTING, book.json.

  • We have standardized on Go Module for vending all the required dependancies. It looks like that what you have started to do, but there’s still a Gopkg.lock and Gopkg.toml which are created by “dep”, these should be removed.

Let me know if you have any questions! We look forward to releasing this with you.

shamsway commented

Thanks @cgriggs01! We'll get to work on this.

shamsway commented

@cgriggs01 I'll send you an email as well, but I think we've completed this initial effort. We're not planning on including CloudOut/CloudOn functionality in this branch so we can get through the initial certification process. I believe the plan is to add those in as quickly as possible, so we will include acceptance tests. We will probably need some assistance figuring out exactly how to accomplish that, but we can discuss that later.

Regarding the website docs, it looks like the "website-test" tool is disabled at the moment, so I wasn't able to do that (source: https://groups.google.com/forum/#!topic/terraform-tool/5xhNxRKvMps). The docs look good to me and I had a team member review them as well, but it's possible there will be some clean-up after we're able to use that tool to view them.

cgriggs01 commented

Hi Team, thanks for the work and changes you’ve made thus far. I just a few more items to address.

  • We use the changelog-links.sh during the build and release process. This is used to update the changelog.md and then a bot publishing a new release tag to the GitHub repo. Because of this, you’ll need to add the next versioned release of the provider. When you add the next version number the date must be “Unreleased” so a bot and this script can add the date during the release. Its up to you what version number you’d like to use but it should look something like 0.1.1 (Unreleased)

  • We try to use the same name for the file with the Go code. Could you change the directory rubrikcdm/ to rubrik/

  • The website documentation is coming together well, your rubrikcdm.erb layout file up one directory into the website/ folder. The name should be changed to rubrik.erb to align naming convention.

  • There is a standard README.md for of the official provider, see Null provider README.md

  • The script build.s should be removed as it won’t be utilized in the Terraform build process.

  • Have you made any progress on a testing environment so we can run make testacc? This will be important that we can automated the testing of the provider from our TeamCity environment