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

Add output for id with camel case #139

Closed
wants to merge 7 commits into from
Closed

Conversation

ByJacob
Copy link

@ByJacob ByJacob commented Feb 7, 2023

what

  • Sometimes a name with only alphanumeric characters is required
  • Using an empty delimiter will generate an 'ugly' string: thisisexamplename
  • A string would look better: thisIsExampleName
  • But achieving such a string requires several additional operations

why

  • It will provide output that can be used immediately for objects that require an alphanumeric name.

references

@ByJacob ByJacob requested a review from a team as a code owner February 7, 2023 08:59
@max-lobur max-lobur requested review from a team as code owners May 17, 2023 14:29
@joe-niland joe-niland requested a review from Nuru July 5, 2023 21:25
outputs.tf Outdated
@@ -8,6 +8,11 @@ output "id_full" {
description = "ID string not restricted in length"
}

output "id_camel_case" {
value = local.enabled ? local.id_camel_case : ""
description = "Disambiguated ID string restricted to `id_length_limit` characters in total and with only alphanumeric characters"
Copy link
Member

Choose a reason for hiding this comment

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

I think this should mention camel case also.

Copy link
Author

Choose a reason for hiding this comment

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

Hey, I've added a usage example along with a link. Will this be sufficient ?

Copy link
Contributor

@Nuru Nuru left a comment

Choose a reason for hiding this comment

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

@ByJacob Thank you very much for this PR.

Because context.tf is in every one of our Terraform modules and components (over 200), and any change affects all of them, we are extremely conservative about making changes. For this reason and others, while I understand your use case, I would oppose adding a new output, such as id_camel_case.

However, you still have options. While it is not well documented (though it is mentioned in the description of label_value_case), the id output is based on the Label Values after the label_value_case is applied. So if you set up null-label like this:

module "pascal" {
  source  = "cloudposse/label/null"
  version = "0.25.0"

  delimiter           = ""
  regex_replace_chars = "/[^a-zA-Z0-9]/"
  label_value_case    = "title"

  context = module.this.context # if you are chaining contexts
}

Then the id will be Pascal case, like ThisIsExampleName. You also get the other outputs working as expected, in particular id_full, so you have your choice of length-limited or not.

There is not (as far as I can see) an elegant way to get camelCase rather than PascalCase out of Terraform. I see you found that out trying to put it together yourself. (I'm not positive, but I expect your code produces PascalCase when label_value_case is upper.) This is why the module does not currently support camelCase. How important is it to you that the first character be lower case?

@ByJacob
Copy link
Author

ByJacob commented Aug 1, 2023

Hey,

Indeed, the solution you gave would be sufficient (it's not a problem to swap the size of the first letter in the terraform). I didn't read that much into the descriptions of all the variables :). I think you can close this MR.

@ByJacob ByJacob closed this Aug 1, 2023
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.

5 participants