Open nicholasspencer opened 6 years ago
Hey, yeah, it looks like a bug. We are loading the issue
property from the config file, but the command line processor does not know about it yet and the command line option is configured with Required()
so it just fails when no value is present on the command line. I will see if I can find a solution.
On option might be to provide a custom command to view your branch issue:
custom-commands:
- name: what
alias: w
script: |
jira view $(git symbolic-ref HEAD | sed -e 's,.*/\(.*\),\1,')
then jira what
or jira w
will call jira view $issue
automatically.
That’s definitely a solution but it would be nice to move the ticket into other state via the normal command. I could totally setup other custom commands to do that but you can see the pain that would be.
Yeah, it is not ideal. The Required()
property is on almost all of the commands that take arguments. I need to figure out a better way to deal with, I have a few ideas I can try out.
More than happy to work on a fix or work on a patch in kingpin that adds params that could bypass the required attribute (probably harder to get a PR through)
The current code follows this pattern:
cmd.Arg("ISSUE", "issue id to view").Required().StringVar(&opts.Issue)
I tried using the Default
argument, since opts.Issue
is populate from configs before kingpin is used, but this does not work:
cmd.Arg("ISSUE", "issue id to view").Default(opts.Issue).Required().StringVar(&opts.Issue)
since kingpin does not allow Required
with Default
. So really the code would have to look like:
if opts.Issue == "" {
cmd.Arg("ISSUE", "issue id to view").Required().StringVar(&opts.Issue)
} else {
cmd.Arg("ISSUE", "issue id to view").Default(opts.Issue).StringVar(&opts.Issue)
}
But since I will have to do that about 40 times in the code, I want a better abstraction to encapsulate this logic. Not sure if you are comfortable hacking on Go and/or have any better ideas for this sort of thing.
I’ve done some Go CLI stuff at previous gigs. Agreed that it should be consolidated logic. I’ll give it a whack and open a PR.
btw, I was apparently wrong about the configs being loaded. They are configured to load when the command is finalized (via callback registered in poorly named jiracli.LoadConfigs
) instead of when we configure the command usage (so that all configs for all commands are not parsed before we execute a single command). So effectively kingpin is configured and executed before configs are parsed. There might be some more delayed callback logic we can do, or we could simply remove the Required
property off kingpin and manually validate in each of the callbacks where we call the command functions:
ie change:
func(o *oreo.Client, globals *jiracli.GlobalOptions) error {
return CmdView(o, globals, &opts)
},
to:
func(o *oreo.Client, globals *jiracli.GlobalOptions) error {
if opts.Issue == "" {
log.Fatalf("ISSUE command line option or `issue` property required")
}
return CmdView(o, globals, &opts)
},
We could have some helper function to make this check for us to make it slightly more palatable. None of the options that come to mind are great.
Cool feature! I'm interested too.
I'm currently doing $ jira view $(git_current_branch)
and the name of my branch is like [EE18-1212] my super feature
Glad I found this ticket, I was hoping for some functionality like jira checkout AB-12345
which would set an env var that applied to all other commands
My "extra curricular" time has been gobbled up these past few months. I'll probably have some free time over the holidays.
I‘m also interested. Automatically setting ISSUE
from a environment variable/current Git branch would be really great.
Did someone work on a fix in the meantime? If I find time, I could try fixing it with the information described in the previous comments, but I‘ve never worked with Go so far.
I'm setting up a dynamic config that looks at the current git branch, parses out the issue id and sets that in the config as
issue: ISSUE-XYZ
. If I dump the env I seeJIRA_ISSUE=ISSUE-XZY
but it seems that sinceISSUE
is a required parameter, the command fails.My config looks like: