-
Notifications
You must be signed in to change notification settings - Fork 231
Don't precompile on pub get/upgrade by default #2277
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
Conversation
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 see a possible corner case around packages and executables that doesn't exist. That might be worth exploring, otherwise this LGTM.
lib/src/command/get.dart
Outdated
help: "Precompile executables in immediate dependencies."); | ||
defaultsTo: false, | ||
help: "Precompile executables in immediate dependencies.\n" | ||
"If false executables will be precompiled on first run."); |
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.
nit: Maybe something like:
If not given, executables will be precompiled by 'pub run'
We can also omit this completely.. But flags aren't really set false, they are more like given or not given.. or...
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.
Omitted it.
lib/src/command/upgrade.dart
Outdated
help: "Precompile executables in immediate dependencies."); | ||
defaultsTo: false, | ||
help: "Precompile executables in immediate dependencies.\n" | ||
"If false executables will be precompiled on first run."); |
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.
nit, see other comment.
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.
omitted it
And only precompile the single desired executable on
pub run
.Following the suggestion from the discussion on #1683 (comment) we avoid printing "precompiling executable..." on
pub run
if we don't have a terminal attached. That allowspub run
to be used in scripts without pollutingstdout
but still explains the user where time is going on first run of eg.pub run test
.This PR does not change
pub global
behavior. We probably want to continue recompiling all executables onpub global activate
.This PR still allows --precompile for those who prefer the old behavior.