Reject submit if api token or cert id are empty #756
Conversation
openshift-ci
bot
added
the
approved
|
/lgtm |
bcrochet
added
the
do-not-merge/work-in-progress
bcrochet
removed
the
do-not-merge/work-in-progress
bcrochet
force-pushed
the
fixes/754
branch
2 times, most recently
from
101b195
to
b9555dd
Compare
bcrochet
force-pushed
the
fixes/754
branch
3 times, most recently
from
0a18871
to
b8fb86a
Compare
There was a problem hiding this comment.
We already discussed this offline, but for the record (happy to be in the minority of opinion, of course, and this is not blocking): I just don't feel like this adds a huge amount of value to us.
We effectively special-case a single flag's behavior in a bit of a unique/opaque way, in an effort to ensure that the submit flag wasn't "eaten" by another. This feels a bit weird to do.
It's a small change, and so for that reason, I'm not going to block on that opinion that may not be shared. Few comments, otherwise looks okay.
|
/retest |
Just setting the flags to required on submit doesn't test for whether the flags are actually provided. It only checks for their existence. This code checks that the data is actually provided, and that another flag isn't accidentally accepted as the value. Fixes redhat-openshift-ecosystem#754 Signed-off-by: Brad P. Crochet <brad@redhat.com>
Addressed. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: acornett21, bcrochet, komish, skattoju The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Just setting the flags to required on submit doesn't test for whether
the flags are actually provided. It only checks for their existence.
This code checks that the data is actually provided, and that another
flag isn't accidentally accepted as the value.
Fixes #754
Signed-off-by: Brad P. Crochet brad@redhat.com