-
Notifications
You must be signed in to change notification settings - Fork 166
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
feat: add support for overriding environment label (#824) #975
feat: add support for overriding environment label (#824) #975
Conversation
Is there anything I can do to aid with the progression of this being reviewed? |
Bump. I'm waiting on this to use Tanka in any capacity since this is a giant footgun that will destroy deployments and eat data. Maintainers, any thoughts? |
This change adds support for overriding the environment label using fields on the environment as per grafana#824. To achieve this, the NameLabel() function that generates this needed to be lifted to the Environment struct (from Metadata) to ensure it can access the fields it needs. Additionally, its signature now returns an error as there are ways errors could occur during this generation now that should be surfaced neatly to the user.
a9bc0cd
to
894830a
Compare
LGTM! |
Thanks @DeanBruntThirdfort for this, nice work! |
How do we feel about |
This change looks to add support for the environment label override from fields that #824 requested.
This allows users to alter the label themselves which allows a workaround of the issue described in #824.
As this is my first time contributing, I've not fully tidied this PR up yet as I want to get feedback from the maintainers on whether approach is sound/in-line with the vision of the codebase and if there's anything else I should bear in mind.