spacelift-io / terraform-provider-spacelift

Terraform provider to interact with Spacelift
MIT License
74 stars 28 forks source link

feat: Add support for description on policies #529

Closed mohamed-haidara-cko closed 6 months ago

mohamed-haidara-cko commented 6 months ago

Description of the change

This PR adds support for setting the description of policies.

Type of change

Related issues

N/A

Checklists

Development

Code review

mohamed-haidara-cko commented 6 months ago

The doc check is failing. Seems like the attributes need to be sorted (?)

michieldewilde commented 6 months ago

@mohamed-haidara-cko did you generate the documentation by running:

cd tools
go generate ./...

There's no need to edit the generated markdown files yourself.

mohamed-haidara-cko commented 6 months ago

@mohamed-haidara-cko did you generate the documentation by running:

cd tools
go generate ./...

There's no need to edit the generated markdown files yourself.

Now yes :-)

Was a little bit confused 🤦

michieldewilde commented 6 months ago

@mohamed-haidara-cko moved to #530. Our tests rely on GitHub secrets and don't work with forks.

mohamed-haidara-cko commented 6 months ago

My last commit should fix the test. You can cherry-pick it :-)

marcinwyszynski commented 6 months ago

LOL @mohamed-haidara-cko thanks, I fixed it the same way in the other PR :)

michieldewilde commented 6 months ago

@mohamed-haidara-cko, would it be possible for you to sign your commits? We can't merge until the commits are signed, and we would prefer to not sign your commits.

mohamed-haidara-cko commented 6 months ago

@mohamed-haidara-cko, would it be possible for you to sign your commits? We can't merge until the commits are signed, and we would prefer to not sign your commits.

Done. Squashed my commits with my key

mwasilew2 commented 6 months ago

@marcinwyszynski is conditionally setting the description in the other PR: https://github.com/spacelift-io/terraform-provider-spacelift/blob/b63f6c21390299f5ad97b7fbe615aeaf6ee309e8/spacelift/resource_policy.go#L199 and the fork is just setting it inline in the struct, the same way the other fields are set.

@marcinwyszynski do you think the other fields should be set conditionally as well?

@michieldewilde other than that the change looks good. I think we can merge this PR and if we decide to set all fields conditionally we can do that in a separate PR.

michieldewilde commented 6 months ago

Merged #530. Closing this PR. @mohamed-haidara-cko, thank you for the contribution!