-
Notifications
You must be signed in to change notification settings - Fork 30
Conversation
30541e1
to
6e2d508
Compare
96d2641
to
f364b57
Compare
Signed-off-by: Alper Rifat Ulucinar <[email protected]>
f364b57
to
2c69f37
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I only have a few small issues. Scanned and didn't see any breaking changes.
Please also make sure that make reviewable
passes with the full set of resources.
@@ -101,7 +101,7 @@ jobs: | |||
go-version: ${{ env.GO_VERSION }} | |||
|
|||
- name: Install goimports | |||
run: go install golang.org/x/tools/cmd/goimports | |||
run: cd /tmp/ && go get golang.org/x/tools/cmd/goimports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the problem was making sure go get
doesn't affect the go.mod
, then the following should help.
run: cd /tmp/ && go get golang.org/x/tools/cmd/goimports | |
run: go install golang.org/x/tools/cmd/goimports@v0.1.8 |
I learned this trick yesterday from go help install
.
If the arguments have version suffixes (like @latest or @v1.0.0), "go install"
builds packages in module-aware mode, ignoring the go.mod file in the current
directory or any parent directory, if there is one. This is useful for
installing executables without affecting the dependencies of the main module.
"gopkg.in/alecthomas/kingpin.v2" | ||
ctrl "sigs.k8s.io/controller-runtime" | ||
"sigs.k8s.io/controller-runtime/pkg/log/zap" | ||
|
||
"github.com/crossplane/terrajet/pkg/terraform" | ||
tf "github.com/hashicorp/terraform-provider-aws/xpprovider" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably unintentional.
@@ -1,212 +0,0 @@ | |||
apiVersion: apiextensions.k8s.io/v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you intend to remove this as well?
That issue is fixed now, can we re-include it? |
Is this still active? |
any progress here ? did we need only to bump https://github.com/crossplane-contrib/provider-jet-aws/blob/main/Makefile#L9 ? @ulucinar
we need LBTargetGroup targetType alb which was added in v3.62.0
we can add the example afterwards in #211 |
Hi @haarchri, |
Yes but simpler Change im makefile to 3.62.0 for example is not working as you can See in my latest comment @ulucinar |
Description of your changes
This PR updates Terraform provider version to
v3.71.0
which supports theaws_cloudfront_response_headers_policy
resource. Some existing resources have API changes with this version. I have also tried the minimum required version ofv3.64.0
but we still have some API changes in existing resources, so I went with the latest released version of the Terraform provider in this PR.Note: I had to exclude
aws_elasticache_user
due to crossplane/terrajet#100, for which a fix is under way.Note: I just regenerated the Terraformed (managed) resources. No reconfiguration or no adjustments on the examples have been performed.
I have:
make reviewable test
to ensure this PR is ready for review.How has this code been tested
Ran
make generate
&make build
.