-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 env substitution to pathorcontent
#4327
Add env substitution to pathorcontent
#4327
Conversation
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
b47043f
to
9dce7fc
Compare
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 LGTM, but let's wait for others 👀 too.
@thanos-io/thanos-maintainers this moves pathOrContent to separate reusable go module BUT it also adds environment variable substitution which makes our flags with config accept environment variables within YAML config using $()
|
||
return extflag.RegisterPathOrContent(cmd, fmt.Sprintf("objstore%s.config", suffix), help, required) | ||
opts := []extflag.Option{extflag.WithEnvSubstitution()} | ||
if required { |
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.
👍🏽
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.
LGTM 🚀
Only one nit about the changelog.
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Changes
This PR adds environment variable substitution to
pathorcontent
flag by replacingpkg/extflag/pathorcontent.go
withgit.luolix.top/efficientgo/tools/extkingpin
. This was added as a new module here. Addresses #4293.Verification
Tested locally.