Skip to content
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

fix(core): fix parallel cli argument not default to 3 when not specified #13837

Merged
merged 3 commits into from
Apr 5, 2023
Merged

fix(core): fix parallel cli argument not default to 3 when not specified #13837

merged 3 commits into from
Apr 5, 2023

Conversation

mike-fam
Copy link
Contributor

@mike-fam mike-fam commented Dec 14, 2022

Current Behavior

When the --parallel flag is not specified, nx commands run the targets sequentially.

Expected Behavior

According to the docs, when the --parallel flag is not specified, it should default to 3.

Related Issue(s)

Fixes #13782
BREAKING CHANGE: Default for --parallel is 3

@vercel
Copy link

vercel bot commented Dec 14, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
nx-dev ✅ Ready (Inspect) Visit Preview Dec 15, 2022 at 1:45AM (UTC)

@AgentEnder
Copy link
Member

Hey @mike-fam! Talked this one over with @FrozenPandaz this morning. This would be a breaking change for some folks that are expecting sequential targets, so we are going to target this for the next major version.

In the meantime, the docs are attempting to reflect that if passing only --parallel, instead of say --parallel 5, the default value would be 3. Do you want to open a PR amending that doc to be clearer?

@mike-fam
Copy link
Contributor Author

mike-fam commented Dec 15, 2022

Hey @AgentEnder, I just want to confirm that having parallel=3 by default when no --parallel flag is specified is the behaviour that we want, albeit breaking, right?

I don't want to introduce unnecessary breaking changes. If we want to update the docs to clarify that if the --parallel flag is not set then the projects will be run sequentially, I can do it too.

@AgentEnder
Copy link
Member

I do think it makes sense that way, still need to talk it over with a few folks.

@mike-fam
Copy link
Contributor Author

@AgentEnder Have you got any update on this?

@@ -170,7 +170,8 @@ export function splitArgsIntoNxArgsAndOverrides(
} else if (
args['parallel'] === 'true' ||
args['parallel'] === true ||
args['parallel'] === ''
args['parallel'] === '' ||

Choose a reason for hiding this comment

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

What is your opinion about creating an array with all the available options and calling the include() function to check if args['parallel'] is matching one of the array's items?

what are the benefits in my opinion?

  • Readability (easy to understand what are the available options in one place)
  • Maintainability (you can always add more cases by adding a new item to the array)

@AgentEnder
Copy link
Member

Hey @mike-fam, sorry for the delay. I don't have an update on the post v16 target for this, for now, updating the docs is the best bet.

@AgentEnder AgentEnder removed the target: next major version To be merged for the next major version release label Apr 5, 2023
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nx's default --parallel value not correctly set to 3
3 participants