search

sematext / terraform-provider-sematext

Sematext Terraform Provider
https://sematext.com
Mozilla Public License 2.0
3 stars 1 forks source link

Feature euan initial content #1

Closed hollerloudly closed 4 years ago

hollerloudly commented 4 years ago

MVP for initial internal comments. PR is to develop, not master.

Some debug code in place. Second PR to land in a couple of days to make this publically presentable, including documentation and examples.

Integration acctests in sematext/terraform-provider-sematext currently all pass.

rabbitstack commented 4 years ago

A couple of general comments:

hollerloudly commented 4 years ago

A couple of general comments:

  • you should probably put the generated code under a dedicated package (e.g. gen). Other code should either live under pkg or internal. Of course, this structure is not mandatory but highly recommended. See more: https://github.com/golang-standards/project-layout
  • I assume you'll replace all the fmt.Print instructions with the native log or third-party (logrus) logger package

Note I'm following Terraform convention very closely, they actually make comment that they depart from above. This repo will go through onboarding and acceptance with Hashicorp and live within their public provider registry. I've used the same Layout as all their other providers. I'll drop the generation prior as part of release out to Hashicorp.

Yup, almost all of the fmt.Print etc debug code will be removed as part of tidyups.

None of the other providers are using a logging package, probably to minimize deps. Will review this with Hashicorp as we are still unclear on their CI.

hollerloudly commented 4 years ago

@hollerloudly Sensei should be axed

Axed.

rabbitstack commented 4 years ago

Note I'm following Terraform convention very closely, they actually make comment that they depart from above. This repo will go through onboarding and acceptance with Hashicorp and live within their public provider registry. I've used the same Layout as all their other providers. I'll drop the generation prior as part of release out to Hashicorp.

Roger that.

AWS Terraform provider uses the stdlib log package, so perhaps that's the way to go.