aptible / terraform-provider-aptible

The official Terraform provider for Aptible Deploy
https://registry.terraform.io/providers/aptible/aptible/latest
10 stars 13 forks source link

Setup CI pipeline for Terraform provider #10

Closed robertfairhead closed 4 years ago

robertfairhead commented 4 years ago

This sets up Travis to run the following checks:

Once this is merged, I'll set github to gate merging PRs on the checks passing.

The second commit was to fix issues flagged by the terraform provider linter. If you disagree with any changes there, we can disable the specific check it triggered in the linter.

robertfairhead commented 4 years ago

/cc @almathew

robertfairhead commented 4 years ago

Yeah sorry, that was my bad. I didn't pay enough attention to what I was doing there.

Those pointers could be nil and if so, those dereferences would cause a segfault/panic in the code. See for example: https://play.golang.org/p/Rw48oux5Ukw

To solve this, we have to check each pointer for being nil before we can safely dereference it. And if it is nil, either return an error that can handled up the stack or substitute an approriate "raw" value. For example: https://github.com/AlekSi/pointer/blob/4f2b1f4deff73a28288291183baa1e9dff4a93a5/value.go#L120

I just spaced the "why" of that warning from having worked in pointerless languages for a bit! Nice catch!

The fix is basically to do the nil checks either here or in go-deploy, whichever you think is cleanest. I'll fix up this PR tomorrow with checks here, but if you think go-deploy is better just let me know and I can add there instead.

reggregory commented 4 years ago

I think for now the checks can just be here, but maybe it would still be better to extract it some to go-deploy later on.

robertfairhead commented 4 years ago

@reggregory It should be ready for another look now with those dereferences fixed and the merge conflicts resolved