-
Notifications
You must be signed in to change notification settings - Fork 68
perf: parallelize create dependencies processing #892
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
perf: parallelize create dependencies processing #892
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit ba803fc. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 5 targetsSent with 💌 from NxCloud. |
|
@AgentEnder, Thanks for reviewing. |
If you have time to look at them, absolutely. Things are a bit hectic on my side so I've not had time to check out what happened. The failures on main are not all true failures, there's something weird with Nx cloud that I've also not had time to investigate. The pr failures are true failures though, so we should figure out why. |
|
Those failures look legit now, so that's good 👍 Thanks for getting em fixed up |
|
| const { stdout } = await promisify(execFile)( | ||
| this.cliCommand.command, | ||
| params, | ||
| { | ||
| cwd: this.cwd ?? process.cwd(), | ||
| }, | ||
| ).catch((e) => { | ||
| if ('code' in e && 'stderr' in e) { | ||
| throw new Error( | ||
| `dotnet execution returned status code ${e.code} \n ${e.message}`, | ||
| ); | ||
| } | ||
| throw e; | ||
| }); | ||
| if (stdout.includes('There are no Project to Project')) { | ||
| return ''; | ||
| } | ||
| return stdout; | ||
| } |
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.
Using execFile instead of spawn made the code slightly more readable and hopefully, easier to maintain.
| ) { | ||
| const depGraphFile = join('tmp', 'dep-graph.json'); | ||
| execSync(`npx nx dep-graph --file ${depGraphFile}`, { | ||
| execSync(`npx nx graph --file ${depGraphFile}`, { |
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.
graph is probably more known than dep-graph by Nx newcomers (dep-graph is not documented anymore)
…#892) * feat(dotnet): create getProjectReferencesAsync method * feat(core): run createDependencies' internal tasks asynchronously and concurrently * fix(dotnet): improve DotnetClient output handling in `spawnAsyncAndGetOutput` * refactor(dotnet): fix and simplify `spawnAsyncAndGetOutput` * chore: run prettier on whole code base
1 similar comment
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |




Fixes #886