[eas-cli] Add flag sanitization in updates command #1473
Conversation
|
Size Change: -4.21 kB (0%) Total Size: 40.1 MB
|
Codecov Report
@@ Coverage Diff @@
## main #1473 +/- ##
==========================================
- Coverage 51.16% 51.13% -0.02%
==========================================
Files 450 450
Lines 15483 15492 +9
Branches 3041 3045 +4
==========================================
Hits 7921 7921
- Misses 7549 7558 +9
Partials 13 13
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -577,6 +602,38 @@ export default class UpdatePublish extends EasCommand { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| private sanitizeFlags(flags: RawUpdateFlags): UpdateFlags { | |||
| const nonInteractive = flags['non-interactive'] ?? false; | |||
There was a problem hiding this comment.
| const nonInteractive = flags['non-interactive'] ?? false; | |
| const nonInteractive = flags['non-interactive']; |
(since this is type boolean)
There was a problem hiding this comment.
Sure, no strong opinions on this one. I just added it for clarity; this is the default (just like we do with some other flags). But it is indeed very explicit.
There was a problem hiding this comment.
Ah, the this.getContextAsync requires a boolean for nonInteractive. Since we have to provide a default anyways, I'll revert this change and put it in the santizeFlags method.
|
|
||
| const { auto, branch: branchName, message: updateMessage } = flags; | ||
| if (nonInteractive && !auto && !(branchName && updateMessage)) { | ||
| Errors.error( |
There was a problem hiding this comment.
these exit/error methods (that return never) are pretty oddly-typed. For example, if you pass {exit: false} in as the second argument, it doesn't exit and the typescript types are incorrect.
I made a similar comment on another recent PR (#1427 (comment)) but my general feeling is that these add a bit of overhead for the reviewer since it's not immediately clear just from reading that they halt the procedure (like a throw would). That being said, I see that these are used in this param sanitization pretty consistently, so I guess it's okay here. It still took me a bit longer to review though, so just a data point.
There was a problem hiding this comment.
Yeah, I hate them too. Probably best to use throw new Error. Can follow up in a PR to get rid of this in all of the santizeFlags methods. For now, I just kept the code consistent with what we already have.
…sy values Co-authored-by: Will Schurman <[email protected]>
…y on falsy values" This reverts commit 23ac36d.
…alsy values" This reverts commit 94ae7cc.
byCedric
force-pushed
the
@bycedric/updates/input-refactor
branch
from
48eec6d to
1983d52
Compare
Checklist
/changelog-entry [breaking-change|new-feature|bug-fix|chore] [message]and CHANGELOG.md will be updated automatically.Why
This starts ENG-5926 and is the first of a couple of stacked PRs to clean this command up.
How
RawUpdateFlagsandUpdateFlagsrunAsyncto emphasize entity relationgroup→groupIdmessage→updateMessageTest Plan
eas update(with all flag-variations should work)