-
Notifications
You must be signed in to change notification settings - Fork 696
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
fix: #1231 - align 1-org labels #1232
fix: #1231 - align 1-org labels #1232
Conversation
Possibly merge after the following and sync this branch |
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.
I want to make sure I understand the intent of this PR... Are you saying the value of application_name
should match the environment code?
Currently there is not a consistent convention for application-name
to match the environment code. A few projects in this stage coincidentally do include the "net" environment code, but I don't think we should try to consistently include the env
code in the application name. My reasoning:
- For downstream use cases where I want to query project labels for all things in a given environment code, it's more reliable to query on the
env
label instead of regex matching theapplication_name
label. - In later stages where a given application is deployed across each of d/n/p environments, the same
application_name
is used across all environments. (example). This is useful if I wanted to query all the projects related to a given app across multiple environments.
Thank you for the review eeaton, I ran into this minor change when merging upstream into a downstream branch. I'll do another review on the changes that were done in this yaml be eod. I am quoting from the issue #1231 below - essentially this change aligns one 2 of the changes to label naming that were done previously. Quote https://github.com/terraform-google-modules/terraform-example-foundation/blob/master/1-org/envs/shared/projects.tf#L253
and
|
My rationale is that the The changes you proposed make a partial match for substrings between I'll close this for now but feel free to re-open if you disagree. |
Sounds good, understood, yes I would like to keep this file consistent with the naming convention, thanks eeaton for the review |
As part of #1231
discovered as part of 387 tef upstream sync