-
Notifications
You must be signed in to change notification settings - Fork 211
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: 'auto' option for runPubGetInParallel #731
base: main
Are you sure you want to change the base?
Conversation
Adds the new value of 'auto' to runPubGetInParallel, which is now default and will only run pub get sequentially if it's being run on a CI instance.
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.
Missing tests and updated docs, but other than that it looks good!
Changed the docs to reflect the change, but tests are a bit more complicated cause I don't really understand how things are set up there and how everything interacts with each other just yet. I can take a deeper look into it during the week though. |
@@ -183,7 +183,7 @@ dependencyOverridePaths: | |||
|
|||
Whether to run `pub get` in parallel during bootstrapping. | |||
|
|||
The default is `true`. | |||
The default is `auto`, which fallbacks to `false` if the machine is a CI instance. |
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.
If this change will make into main, I would suggest adding explanation to this line. Why is this the default, what is the issue it resolves, and what could someone who changes this to true expect.
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.
That's a good idea
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.
Can you look into that @tomassasovsky?
@tomassasovsky any updates on this? :) |
Description
Adds the new value of 'auto' to
runPubGetInParallel
, which is now default and will only run pub get sequentially if it's being run on a CI instance.Since the current variable type is of
bool
for this variable, I had to add some code to handle both the old and new possible data types for parsing. Any alternative ideas for this are more than welcome.PS: thanks for making such an awesome tool!
Closes #546
Type of Change
feat
-- New feature (non-breaking change which adds functionality)fix
-- Bug fix (non-breaking change which fixes an issue)!
-- Breaking change (fix or feature that would cause existing functionality to change)refactor
-- Code refactorci
-- Build configuration changedocs
-- Documentationchore
-- Chore