Skip to content

Rename --checked to --check-asserts #1964

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

Merged
merged 3 commits into from
Aug 8, 2018
Merged

Conversation

natebosch
Copy link
Member

Keep the old flag as an alias, run with checked mode if either is set.
Behavior will be weird for cases like --checked --no-check-asserts but
it's not worth the extra complexity to track that down.

Update tests using the flag.

Use consistent single quotes in the files that were being edited anyway.

Keep the old flag as an alias, run with checked mode if either is set.
Behavior will be weird for cases like `--checked --no-check-asserts` but
it's not worth the extra complexity to track that down.

Update tests using the flag.

Use consistent single quotes in the files that were being edited anyway.
@kevmoo
Copy link
Member

kevmoo commented Aug 7, 2018

Shouldn't we be going with --enable-asserts ? To be consistent with...everything else?

@natebosch
Copy link
Member Author

See #1922 (comment)

for consistency with the VM and Dart2Js
@natebosch
Copy link
Member Author

discussed offline. Went with --enable-asserts and update the help string to match dart --help

@natebosch natebosch requested a review from jakemac53 August 8, 2018 20:57
@natebosch natebosch merged commit 9f00679 into master Aug 8, 2018
@natebosch natebosch deleted the renamed-check-asserts branch August 8, 2018 21:55
argParser.addFlag("checked",
abbr: "c", help: "Enable runtime type checks and assertions.");
argParser.addOption("mode", help: "Deprecated option", hide: true);
argParser.addFlag('enable-asserts', help: 'Enable assert statements.');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following seems clearer / more accurate as help text: Enable runtime checks of assert statements. WDYT? /cc @kwalrath @kevmoo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took this directly from dart --help. If we want to change this I think we should do it in both places.

argParser.addFlag("checked",
abbr: "c", help: "Enable runtime type checks and assertions.");
argParser.addOption("mode", help: "Deprecated option", hide: true);
argParser.addFlag('enable-asserts', help: 'Enable assert statements.');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

@@ -22,7 +22,7 @@ import 'utils.dart';
///
/// Arguments from [args] will be passed to the spawned Dart application.
///
/// If [checked] is true, the program is run in checked mode.
/// If [checked] is true, the program is run with assertions enabled.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere: how about "with assertion checking enabled"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants