Conversation
Co-authored-by: Michael Bahr <1830132+bahrmichael@users.noreply.github.com>
| }, | ||
| }) | ||
|
|
||
| func parseABCVariableNames(positional []string, flagged abcVariableArgs) ([]string, error) { |
There was a problem hiding this comment.
think we can collapse this into runABCVariablesDelete
| return variableNames, nil | ||
| } | ||
|
|
||
| func runABCVariablesDelete(ctx context.Context, client api.Client, instanceID string, positional []string, flagged abcVariableArgs, output io.Writer, getCurl bool) error { |
There was a problem hiding this comment.
positional and flagged can be collapsed into one array imo
|
|
||
| func (a *abcVariableArgs) Set(value string) error { | ||
| *a = append(*a, value) | ||
| return nil |
There was a problem hiding this comment.
never return an error so we might as well remove it from the function signature
| return abcVariable{Key: name, Value: value}, nil | ||
| } | ||
|
|
||
| func runABCVariablesSet(ctx context.Context, client api.Client, instanceID string, positional []string, flagged abcVariableArgs, output io.Writer, getCurl bool) error { |
There was a problem hiding this comment.
I think positional + flagged should be combined before this method is called
| return nil, cmderrors.Usage("must provide at least one variable assignment") | ||
| } | ||
|
|
||
| variables := make([]abcVariable, 0, len(rawVariables)) |
There was a problem hiding this comment.
I think you can use strings.FieldsFunc
There was a problem hiding this comment.
If I understand it correctly urfave should already give us individual args that don't need to be split by whitespace. For each arg (name=value) the strings.Cut method looks like a better fit to me.
| return err | ||
| } | ||
|
|
||
| if getCurl { |
There was a problem hiding this comment.
I think if you're not going to support this flag then we should:
- not pass it in
- print a warning before running this that
get-curlis currently unsupported for this command
| } | ||
|
|
||
| if getCurl { | ||
| return nil |
There was a problem hiding this comment.
see my later comment on get-curl
2 participants
This PR introduces a
src abc variablescommand for setting and deleting variables on workflow instances using the update variables api from Sourcegraph.The workflow instance id is an argument after the delete/set command, so that we can follow the existing subcommand patterns without having to introduce support for prefix args.