feat: Raise an error when invalid custom field is specified. #518
Conversation
There was a problem hiding this comment.
Hi @martinpovolny, thank you for the PR.
I think the validation was not added because the Jira API is flexible about this. The API simply ignores invalid fields and set the valid ones. The CLI behavior is same as this and I am not sure if we need to be strict on the CLI side. Also, this would be sort of a breaking change IMO. Thoughts?
pkg/jira/create.go
Outdated
|
|
||
| if !found { | ||
| fmt.Fprintf(os.Stderr, "\nInvalid custom field specified: %s\n", key) | ||
| os.Exit(1) |
There was a problem hiding this comment.
Since this is a package, instead of exiting we should return appropriate error and let the caller decide what to do with the error.
martinpovolny
force-pushed
the
invalid_custom_fields
branch
from
f334f2c
to
8e90aba
Compare
The problem is that when you (as a user) have incorrectly set Custom fields in the configuration file, you have no idea, what is happening because the invalid custom field is silently ignored. That is confusing and creates frustration when using the Custom fields feature. It took me several tries and several minutes to figure out what was wrong in my case. At the very least there should be a warning. You are passing an argument and expect it to have an effect. There's none. I would not consider it a breaking change when the CLI newly checks this. It merely points out an error situation that was previously wrongly ignored. |
martinpovolny
force-pushed
the
invalid_custom_fields
branch
from
8e90aba
to
1f59ff5
Compare
martinpovolny
force-pushed
the
invalid_custom_fields
branch
from
1f59ff5
to
efb99e5
Compare
I think we should start with the warning and fail in future release. I would suggest the following:
|
Raise an error when an invalid custom field is specified.
Currently, the invalid custom field is silently ignored which creates a wrong impressing that everything is OK.