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

types.reference: calculate reference field name correctly #48

Merged
merged 1 commit into from
Jul 29, 2022
Merged

types.reference: calculate reference field name correctly #48

merged 1 commit into from
Jul 29, 2022

Conversation

muvaf
Copy link
Member

@muvaf muvaf commented Jul 28, 2022

Description of your changes

This is currently blocking https://github.com/upbound/official-providers/pull/457 to go through.

Fixes #45

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

In https://github.com/upbound/official-providers/pull/457 , I've confirmed that the ref tags are produced as apiIdRef instead of apiidRef.

However, there is another change in some resources in AWS. In cases we give custom ref and selector field names, like VpcSecurityGroupIdRefs so that it doesn't end up being VpcSecurityGroupIdsRefs, we run into errors since with the new calculation we always default to using .Camel property of the name, which is in line with the CRD type builder but those custom overrides are given as camel computed, i.e. given as VpcSecurityGroupIdRefs instead of VPCSecurityGroupIDRefs. What we end up with without this change is the following:
image

With this change we produce the fields with .Camel but then the reference comments don't match. So, we should either opt for .CamelComputed for all fields or change the custom overrides and I'm leaning towards the latter. Azure and GCP are not affected since apparently they haven't overridden any ref or selector fields.

image

…t that originally has snake case so that calculation is correct for cases where there are multiple acronyms one after the other

Signed-off-by: Muvaffak Onus <[email protected]>
Copy link
Member

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

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

LGTM!

@muvaf muvaf merged commit 037f1fc into crossplane:main Jul 29, 2022
@muvaf muvaf deleted the ref-name-calc branch July 29, 2022 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reference field tag names are incorrectly calculated when there are multiple acronyms back to back
2 participants