Sub-commands (inline variable setting) #5
Conversation
README.md
Outdated
|
|
||
| ```sh | ||
| $ glci sv --token <gitlab-token> --url <gitlab-project-url> --key <gitlab-key> --value <gitlab-value> | ||
| Set NPM_INSTALL_TOKEN = <value> for gitlab-org/gitlab-ce. |
There was a problem hiding this comment.
I think this line will be Set <key> = <value>...
README.md
Outdated
| - `gitlab-value` is the value of the variable you want to set | ||
|
|
||
| ```sh | ||
| $ glci sv --token <gitlab-token> --url <gitlab-project-url> --key <gitlab-key> --value <gitlab-value> |
There was a problem hiding this comment.
Below we just use key, value, so perhaps the placeholders could be updated to reflect the same.
README.md
Outdated
| $ export GITLAB_TOKEN=this-is-my-gitlab-token | ||
| $ glci sv --url <gitlab-project-url> --key <gitlab-key> --value <gitlab-value> | ||
| Using token from environment variable GITLAB_TOKEN. | ||
| Set AWS_CREDENTIALS = <value> for gitlab-org/gitlab-ce. |
README.md
Outdated
| $ glci sv --key <gitlab-key> --value <gitlab-value> | ||
| Using token from environment variable GITLAB_TOKEN. | ||
| No URL specified, using git remote `origin`. | ||
| Set NPM_INSTALL_TOKEN = <value> for gitlab-org/gitlab-ce. |
| }, | ||
| "bin": { | ||
| "setAllVars": "bin/setAllVariables.js" | ||
| "glci": "bin/glci.js" |
There was a problem hiding this comment.
👍
To retain BC, could we leave the old setAllVars as well?
Or, if we don't feel like that's important, the tool could just go to v2?
There was a problem hiding this comment.
I suggest going v2
|
|
||
| // Check for token | ||
| if (!conf.token) { | ||
| if (!process.env.GITLAB_TOKEN) { |
There was a problem hiding this comment.
We shouldn't need this, the rc package will automatically look for environment variables:
environment variables prefixed with ${appname}_
I feel like that'd mean GITLAB_TOKEN would already register... unless it's case sensitive and looking for gitlab_token. Did you have problems without this code?
There was a problem hiding this comment.
It is case sensitive, e.g. if env variable GITLAB_TOKEN is set, it would be exposed as conf.TOKEN
There was a problem hiding this comment.
I didn't know rc could do that. However, doing so will lead to not knowing from where comes the token, command arguments or environment variable. Don't you think this is a problem ?
There was a problem hiding this comment.
I don't think it's a problem, but I'm happy to support it.
There was a problem hiding this comment.
Agreed with Baptiste. Doing so saves some lines code, but takes more brain power to understand for the next contributor.
src/lib/utils.js
Outdated
|
|
||
| const gitlabEnvFileName = 'gitlab.env.yml'; | ||
|
|
||
| async function getConf(cmd, bSingle = false) { |
There was a problem hiding this comment.
Having cmd here creates tight coupling between this utility method and program. To reduce coupling, we could pass doNotForce as a boolean argument, or options as an object argument, e.g. { doNotForce: true}
src/lib/utils.js
Outdated
| } | ||
| } | ||
|
|
||
| if (bSingle) { |
There was a problem hiding this comment.
This logic does not belong here. A property (i.e. key/value combination) is not part of config (i.e. information on how to connect to gitlab and how to handle existing values). This logic belongs in glci-sv.js.
+ put convenience functions in a separate file (utils.js)
|
|
||
| // Check for token | ||
| if (!conf.token) { | ||
| if (!process.env.GITLAB_TOKEN) { |
There was a problem hiding this comment.
I don't think it's a problem, but I'm happy to support it.
| }, | ||
| "bin": { | ||
| "setAllVars": "bin/setAllVariables.js" | ||
| "glci": "bin/glci.js" |
3 participants
gitlab.env.yml.tokencan be retrieved from env variable.glci sv <options>to set one variable.glci sav <options>to set all variables ingitlab.env.yml.see README here : https://github.com/bredondev/gitlab-ci-variables-cli/tree/onevar