-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: validate ncu-ci args #411
Conversation
425fab1
to
6408ccd
Compare
6408ccd
to
4cba1ff
Compare
Codecov Report
@@ Coverage Diff @@
## master #411 +/- ##
=======================================
Coverage 76.28% 76.28%
=======================================
Files 21 21
Lines 1480 1480
=======================================
Hits 1129 1129
Misses 351 351 Continue to review full report at Codecov.
|
}).check(argv => { | ||
if (argv.markdown && commandKeys.includes(argv.markdown)) { | ||
throw new Error('--markdown <path> did not specify a valid path'); | ||
} else if (argv.json && commandKeys.includes(argv.json)) { | ||
throw new Error('--json <path> did not specify a valid path'); | ||
} | ||
return true; | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use requiresArg: true
in the option's definition instead of manual validation
@targos i tried that initially but it didn't seem to have the desired effect - it just silently fails as before:
if i specify:
with no manual validation Looks like there might be a bug: yargs/yargs#1041 |
Closes #393.
Not a perfect solution, but this validates that the user didn't type something like:
and throws a helpful error if that's the case.
cc @mmarchini @joyeecheung