fix(envconfig): rename CLUSTER_DOMAIN env var to FLAGD_CLUSTER_DOMAIN #792
Conversation
Add CLUSTER_DOMAIN environment variable to allow customization of the Kubernetes cluster domain used in flagd-proxy service URIs. The default value remains "cluster.local" to maintain backward compatibility. This enables support for clusters using non-standard domain configurations. Signed-off-by: Roman Kuznetsov <[email protected]>
Prefix the cluster domain environment variable with FLAGD_ to maintain consistency with other flagd-related configuration options. Signed-off-by: Roman Kuznetsov <[email protected]>
Summary of ChangesHello @kuznero, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the consistency of environment variable naming within the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request renames the CLUSTER_DOMAIN environment variable to FLAGD_CLUSTER_DOMAIN to improve consistency with other flagd-related configuration options. The change is valid, however, to further improve consistency within the EnvConfig struct itself, I've suggested renaming the struct field from ClusterDomain to FlagdClusterDomain to align with its corresponding environment variable.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #792 +/- ##
===========================================
- Coverage 86.51% 72.33% -14.19%
===========================================
Files 19 30 +11
Lines 1587 1923 +336
===========================================
+ Hits 1373 1391 +18
- Misses 173 484 +311
- Partials 41 48 +7
... and 26 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Rename the struct field from ClusterDomain to FlagdClusterDomain to maintain consistency with its environment variable FLAGD_CLUSTER_DOMAIN and other flagd-related configuration options in the EnvConfig struct. Signed-off-by: Roman Kuznetsov <[email protected]>
Revert the field rename in FlagdProxyConfiguration struct since its other fields (Port, Image, Tag, etc.) don't use the Flagd prefix - the prefix is already in the struct name itself. Only EnvConfig uses the FlagdClusterDomain naming to match other env config fields like FlagdProxyPort, FlagdProxyImage, etc. Signed-off-by: Roman Kuznetsov <[email protected]>
Signed-off-by: Roman Kuznetsov <[email protected]>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly renames the CLUSTER_DOMAIN environment variable to FLAGD_CLUSTER_DOMAIN for consistency, updating the EnvConfig struct and its usages. The changes are correct and align with the PR's goal. My review includes one comment on a related field that appears to be unused, suggesting a cleanup to improve code maintainability.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly renames the CLUSTER_DOMAIN environment variable to FLAGD_CLUSTER_DOMAIN to improve consistency. The changes are applied correctly across the configuration, implementation, and test files. I've added one comment regarding potentially unused code that is being touched by this change, which could be an opportunity for a small cleanup.
1 participant
Summary
FLAGD_to maintain consistency with other flagd-related configuration optionsContext
This is a follow-up to #789 which added configurable cluster domain support. The environment variable naming should be consistent with other flagd-related options.
Changes
CLUSTER_DOMAINtoFLAGD_CLUSTER_DOMAINin envconfigSigned-off-by: Roman Kuznetsov [email protected]