Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add aliases for settings names #66

Closed
ErinCall opened this issue Jan 7, 2020 · 0 comments · Fixed by #71
Closed

Add aliases for settings names #66

ErinCall opened this issue Jan 7, 2020 · 0 comments · Fixed by #71
Labels
enhancement New feature or request

Comments

@ErinCall
Copy link
Contributor

ErinCall commented Jan 7, 2020

The problem I'm trying to solve:

Some of the configuration fields' names aren't great. helm_command, is a leaky abstraction, for example. Most of them are for drone-helm2 backwards-compatibility; some might just be because I didn't think the names through.

How I imagine it working:

In internal/helm.NewConfig, look for environment variables with alternate names for some settings. For example, we could allow PLUGIN_MODE or maybe PLUGIN_OPERATION in place of PLUGIN_HELM_COMMAND.

The docs should probably note the "better" name as the main setting name, and note the "worse" name as a backwards-compatibility alias.

We'll need to figure out what to do if both versions are present--error, default to one or the other, something else...?
@ErinCall ErinCall added the enhancement New feature or request label Jan 7, 2020
ErinCall added a commit that referenced this issue Jan 8, 2020
@ErinCall
Rough draft of aliased settings [#66]
04de280
ErinCall added a commit that referenced this issue Jan 8, 2020
@ErinCall
Use "kube" in setting aliases [#66]
1d1117b 
Nobody likes typing "kubernetes"! Writing out that whole word without
typos is the third hard problem in computer science.
ErinCall added a commit that referenced this issue Jan 8, 2020
@ErinCall
Stabilize the logic for setting-alias conflicts [#66]
6aa1d79 
This includes a refactor to the way aliases are processed. I had been
thinking in terms of locking down the aliases names pretty tightly, in
order to provide an error if there are conflicts. After discussion with
@josmo, though, it seems like we can do it the same way we do for
"PLUGIN_"/non-prefixed variables, i.e. quietly override them.
ErinCall added a commit that referenced this issue Jan 8, 2020
@ErinCall
Default to the canonical setting names [#66]
a5342d7 
The goal with these changes was to give users a clearer, more readable
interface, so we should present that interface up front and only
document the aliases as a backward-compatibility option.

I've renamed the envconfig tags to reflect the switch, but I left the
actual field names the same. I think they're sufficiently meaningful
inside the code, and leaving them unchanged avoids making a bunch of
churn in the rest of the code.
@ErinCall ErinCall mentioned this issue Jan 8, 2020
Merged
6 tasks
ErinCall added a commit that referenced this issue Jan 9, 2020
@ErinCall
Test settings aliases without the plugin_ prefix [#66]
a826f66
ErinCall added a commit that referenced this issue Jan 9, 2020
@ErinCall
Verify that every config field gets populated [#66]
b4b1f78 
I'm about to radically alter the way NewConfig populates its fields, so
I want assurance that it's still working correctly afterward.
ErinCall added a commit that referenced this issue Jan 9, 2020
@ErinCall
Get env vars from urfave/cli instead of envconfig [#66]
7892c9c
ErinCall added a commit that referenced this issue Jan 9, 2020
@ErinCall
Mention aliased settings in parameter_reference [#66]
3d1a222
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use clearer setting names, with backward-compatibility aliases pelotech/drone-helm3
1 participant
@ErinCall