-
Notifications
You must be signed in to change notification settings - Fork 147
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
feat(shorebird_cli): add --dry-run
and --force
to shorebird patch
#207
feat(shorebird_cli): add --dry-run
and --force
to shorebird patch
#207
Conversation
--dry-run
and --force
to shorebird patch
@@ -94,6 +106,14 @@ class PatchCommand extends ShorebirdCommand | |||
return ExitCode.software.code; | |||
} | |||
|
|||
final force = results['force'] == 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.
Looks fine, but isn't this also solved by adding default: false to the addFlag?
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.
Then it's just results['force']? Donno which is clearer. 🤷
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 tried this – results['force']
is still null if no arg is provided.
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.
default: false
is the default but accessing arg flags is dynamic
which is why I wrote it this way.
@@ -433,9 +452,6 @@ Please create a release using "shorebird release" and try again. | |||
|
|||
test('throws error when release artifact cannot be retrieved.', () async { | |||
const error = 'something went wrong'; | |||
when( |
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.
Why are these no longer needed?
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.
They are the default stubs so they were unnecessary
Description
Type of Change