cockroachdb / terraform-provider-cockroach

Terraform provider for CockroachDB Cloud
Apache License 2.0
57 stars 12 forks source link

Added github actions and fixed go-lint issues #8

Closed abhishekdwivedi3060 closed 2 years ago

abhishekdwivedi3060 commented 2 years ago
marksoper commented 2 years ago

@abhishekdwivedi3060 should we add something to the README.md to explain CI/CD?

abhishekdwivedi3060 commented 2 years ago

@abhishekdwivedi3060 should we add something to the README.md to explain CI/CD?

We are going to add CI flow once we have the CI ready and running. At this stage, we are not yet clear about what all jobs are going to be there.

marksoper commented 2 years ago

@abhishekdwivedi3060 I don't see the ability to review/approve. I'm ok with merging this, but looks like checks are failing. Should I go ahead and merge?

abhishekdwivedi3060 commented 2 years ago

@abhishekdwivedi3060 I don't see the ability to review/approve. I'm ok with merging this, but looks like checks are failing. Should I go ahead and merge?

Let me fix those first.

abhishekdwivedi3060 commented 2 years ago

@marksoper Problem is that TF provider uses SDK which is a private repo so CI jobs are not able to pull that dependency vendor and hence failing. For now I've put dependency in the repo itself and once we make the SDK public we can remove the vendor directory

abhishekdwivedi3060 commented 2 years ago

UnitTest failure is expected as we don't have test-cases as of now. If rest of the code looks good then we can merge this

marksoper commented 2 years ago

@abhishekdwivedi3060 the number of files changed has jumped up to 1800+ Was this intended?

image
abhishekdwivedi3060 commented 2 years ago

@abhishekdwivedi3060 the number of files changed has jumped up to 1800+ Was this intended? image

Yes because SDK is a private repo, as a result of which TF CI is not able to pull that dependency, so had to add all dependency in the repo itself. This will be removed once we make the repo public. We can ignore the vendor directory all together. That is managed by Golan