CiscoDevNet / terraform-provider-aci

Terraform Cisco ACI provider
https://registry.terraform.io/providers/CiscoDevNet/aci/latest/docs
Mozilla Public License 2.0
84 stars 99 forks source link

[minor_change] Add datasource and resource for fvFBRoute in aci_vrf_f… #1227

Open akinross opened 4 weeks ago

akinross commented 4 weeks ago

…allback_route and aci_vrf_fallback_route_group

fixes #1185

akinross commented 4 weeks ago

CI failing because of introduced dependency in main.go, please ignore during review asked @samiib already to have a look

codecov-commenter commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 89.97093% with 69 lines in your changes missing coverage. Please review.

Project coverage is 87.15%. Comparing base (5f5cc63) to head (1f32250).

Files Patch % Lines
...ternal/provider/resource_aci_vrf_fallback_route.go 86.51% 35 Missing and 18 partials :warning:
...nal/provider/data_source_aci_vrf_fallback_route.go 92.30% 6 Missing and 2 partials :warning:
.../provider/resource_aci_vrf_fallback_route_group.go 93.38% 5 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1227 +/- ## ========================================== + Coverage 87.03% 87.15% +0.12% ========================================== Files 43 45 +2 Lines 11223 11867 +644 ========================================== + Hits 9768 10343 +575 - Misses 991 1037 +46 - Partials 464 487 +23 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

samiib commented 4 weeks ago

CI failing because of introduced dependency in main.go, please ignore during review asked @samiib already to have a look

@akinross

I tried adding the new dependency as an import in generator.go to force it to be downloaded to vendor but that wasn't working. See: https://github.com/CiscoDevNet/terraform-provider-aci/commit/edfdf7b4809d427528b723022cd72e7f9ea2bc9d import "golang.org/x/tools/cmd/goimports" is a program, not an importable package

I don't see any other way to get this into vendor as it's not really intended to be used to vendor programs. The vendor command will only find packages required to build the provider and other programs are not really required to build it, just generate it.

For now I got around this by adding -mod=mod to the single generate command.

@lhercot - This may be the one exception to having everything in vendor. What do you think?

samiib commented 2 weeks ago

CI failing because of introduced dependency in main.go, please ignore during review asked @samiib already to have a look

@akinross

I tried adding the new dependency as an import in generator.go to force it to be downloaded to vendor but that wasn't working. See: edfdf7b import "golang.org/x/tools/cmd/goimports" is a program, not an importable package

I don't see any other way to get this into vendor as it's not really intended to be used to vendor programs. The vendor command will only find packages required to build the provider and other programs are not really required to build it, just generate it.

For now I got around this by adding -mod=mod to the single generate command.

@lhercot - This may be the one exception to having everything in vendor. What do you think?

Thank you @lhercot for suggesting a different method of specifying the tool dependency in the provider. I committed the change and the vendor files to the PR and this is working well in the CI now.