refactor: Simplify CLI flag validation #1699
Conversation
|
rschristian
force-pushed
the
refactor/invalid-args
branch
from
8b677aa
to
8fef389
Compare
rschristian
force-pushed
the
refactor/invalid-args
branch
from
2287be2
to
be4d2be
Compare
rschristian
changed the title
rschristian
changed the title
rschristian
force-pushed
the
refactor/invalid-args
branch
2 times, most recently
from
ee68207
to
80a1037
Compare
rschristian
force-pushed
the
refactor/invalid-args
branch
3 times, most recently
from
84e8307
to
c28c4fd
Compare
rschristian
changed the title
packages/cli/lib/index.js
Outdated
| }); | ||
| buildCommand.action(commands.build); | ||
| ) | ||
| .option('--src', 'Specify source directory', 'src') |
There was a problem hiding this comment.
I think I remember it originally being done this way, but I don't know why it was changed to the version it currently is
Probably older than the new sade option
There was a problem hiding this comment.
Indeed, we had the commands in this format but I think sade's option was just missed, so validation was done the way it was. It's a pretty long ReadMe without a table of contents, easy enough to do.
Alternatively, maybe there was an idea for better type checking on args down the road? sade/mri are a bit "loose" with coercing input types, leading to a few odd situations. Just guessing though; if we wanted to do that there's probably simpler ways to validate the schema than this setup.
rschristian
force-pushed
the
refactor/invalid-args
branch
from
8823d5e
to
7b6b007
Compare
What kind of change does this PR introduce?
Refactor
Did you add tests for your changes?
Adjusted one, yes.
Larger test changes were extracted to #1738 because this diff was rather large and I wanted to use base other work off of both of these independently.
Summary
Partial revert of #1467
sadehas a builtin mechanism for catching unknown options being passed to it, allowing us to simplify things a bit.There should be no functional changes here at all.
Does this PR introduce a breaking change?
No