Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to latest ACK version to make the pipeline work again #993

Closed

Conversation

muvaf
Copy link
Member

@muvaf muvaf commented Dec 8, 2021

Description of your changes

After a few fixes in code-generator that make the Crossplane integration work again (thanks @AaronME and @haarchri !) , #992 and a few fixes in the first commit of this PR, we are able to update code-generator to the latest version.

Note to reviewers, only the first commit contains handwritten changes.

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

make reviewable

…es to make it work with Crossplane again

Signed-off-by: Muvaffak Onus <me@muvaf.com>
Signed-off-by: Muvaffak Onus <me@muvaf.com>
type S3OriginConfig struct {
OriginAccessIDentity *string `json:"originAccessIDentity,omitempty"`
OriginAccessIdentity *string `json:"originAccessIdentity,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the above will be breaking changes. It's an alpha API so we can do that, but it's probably worth a prominent release note at least.

type ModifyVPNTunnelOptionsSpecification struct {
DPDTimeoutAction *string `json:"dPDTimeoutAction,omitempty"`
DPDTimeoutAction *string `json:"dpdTimeoutAction,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we don't generate the CRD that was supposed to use this struct, hence it doesn't show up in any CRDs.

@@ -18,46 +18,6 @@ spec:
singular: address
scope: Cluster
versions:
- name: v1alpha1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we expect these versions to disappear?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@muvaf same question - in a few ec2 crds stuff is removed is this expected ? the rest in this PR looks good - tested also a few resources

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we added the skipversion tags to get those removed. See #876 for details.

@muvaf
Copy link
Member Author

muvaf commented Dec 8, 2021

Closing this in favor of @AaronME 's #920 . I swear I looked for that PR and couldn't find it but then saw it while I was looking for #876 . The PR is exactly the same I believe, so same review comments apply there as well.

@muvaf muvaf closed this Dec 8, 2021
@muvaf muvaf deleted the make-generator-great-again branch December 8, 2021 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants