-
Notifications
You must be signed in to change notification settings - Fork 43
chore: extract option defaults to options package #283
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
Conversation
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!
// Temporarily removed these from the default settings to prevent conflicts | ||
// between current and legacy environment variables that add default values. | ||
// Once the legacy environment variables are phased out, this can be | ||
// reinstated to the previous default values. |
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 know this was just moved and logic didn't change, but I wonder what this comment means. 🤔
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.
Related to ENVBUILDER_
env prefix.
Signed-off-by: Cian Johnston <[email protected]>
Signed-off-by: Cian Johnston <[email protected]>
Signed-off-by: Cian Johnston <[email protected]>
4df4433
to
fd6ef17
Compare
Signed-off-by: Cian Johnston <[email protected]> (cherry picked from commit 09ce456)
Part of coder/internal#11
Builds on top of #282
More refactoring: