refactor!: change envVars to env in func.yaml #316
Conversation
| @@ -22,7 +22,7 @@ type config struct { | |||
| Trigger string `yaml:"trigger"` | |||
| Builder string `yaml:"builder"` | |||
| BuilderMap map[string]string `yaml:"builderMap"` | |||
| EnvVars map[string]string `yaml:"envVars"` | |||
| Env map[string]string `yaml:"env"` | |||
There was a problem hiding this comment.
@lance won't this cause issues for users using newer versions with older conf file?
There was a problem hiding this comment.
But not many people use function so it is not that much of a problem. We just need to let know people that it's been changed.
There was a problem hiding this comment.
Yes - you are right that any existing users would need to change their config file. But this is all so new, and it's still unsupported, so I think perhaps adding to release notes for TP-2 would be OK instead of trying for backwards compatibility. But I will follow up on this to be sure it's OK.
There was a problem hiding this comment.
LGTM!
Yes, this is technically a backwards incompatible change, but let's get this thing squeaky-clean now, before 1.0, while we still can. (I also doubt there are others using env vars other than we team members at this point?),
lance
force-pushed
the
lance/315-change-env-vars
branch
from
7ad2b60 to
43eedb0
Compare
|
I should have noted that this change was prompted by the discussion in #312 (comment). |
| @@ -22,7 +22,7 @@ type config struct { | |||
| Trigger string `yaml:"trigger"` | |||
| Builder string `yaml:"builder"` | |||
| BuilderMap map[string]string `yaml:"builderMap"` | |||
| EnvVars map[string]string `yaml:"envVars"` | |||
| Env map[string]string `yaml:"env"` | |||
There was a problem hiding this comment.
LGTM!
Yes, this is technically a backwards incompatible change, but let's get this thing squeaky-clean now, before 1.0, while we still can. (I also doubt there are others using env vars other than we team members at this point?),
|
Nit: imho this shouldn't be a |
Good point! |
There are some places where I've changed variable and function names where it wasn't strictly necessary. If you don't mind the bit of churn that results, changing these makes `rg -i envvars` a nice way to check for anything that could be overlooked. Signed-off-by: Lance Ball <lball@redhat.com>
lance
force-pushed
the
lance/315-change-env-vars
branch
from
43eedb0 to
69f9710
Compare
lance
changed the title
|
@zroubalik how about |
There are some places where I've changed variable and function names
where it wasn't strictly necessary. If you don't mind the bit of churn
that results, changing these makes
rg -i envvarsa nice way to checkfor anything that could be overlooked.
Signed-off-by: Lance Ball lball@redhat.com