-
Notifications
You must be signed in to change notification settings - Fork 2
feat(stack_name_template): allow reformatting stack ids #101
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
base: main
Are you sure you want to change the base?
Conversation
| # Following source doesn't work in most setups | ||
| ignored: | ||
| - SC1090 | ||
| - SC1091 |
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.
this was generated by trunk when I ran trunk upgrade
Gowiem
left a comment
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.
@dudymas this looks great overall, well done!
I have one issue I want to work through before we move this forward and it's the problem of supporting nested directories in this child module, which I'm starting to regret as I believe that is adding some unnecessary complexity in multiple areas of our code.
I'd like to chat with you and @oycyc who originally introduced that idea and see if that is actually an important feature or something we can remove / not continue to build workarounds for. Let's chat about that tomorrow and keep this PR on hold until we discuss -- Thanks!
| module => { | ||
| for file, content in files : | ||
| file => ( | ||
| file == var.common_config_file ? null : |
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.
Do you need this check if you have the if file != var.common_config_file on the for?
|
@dudymas @oycyc as a follow up to our retro earlier today, I want to go full circle on this PR and introduce a breaking change that fixes this bad name formatting instead of just introducing a backwards compatible template variable that we can use to fix it. I would like to do that next week alongside @gberenice's PR (#104) which I think should also introduce a breaking change. I'll schedule 30 minutes next week so the 4 of us can get on the same page and then execute what we need to do! |
what
why