Skip to content

Conversation

samueltardieu
Copy link
Member

@samueltardieu samueltardieu commented Mar 5, 2023

Use multithreading unless there is a reason not to.

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2023

r? @dswij

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 5, 2023
@samueltardieu
Copy link
Member Author

r? @llogiq
(as you dealt with other lintcheck changes that I proposed recently)

@rustbot rustbot assigned llogiq and unassigned dswij Mar 8, 2023
@llogiq
Copy link
Contributor

llogiq commented Mar 8, 2023

I don't think we need an Option for max_jobs. Otherwise this looks ok to me.

Comment on lines 59 to 63
config.max_jobs = Some(if config.fix || config.recursive {
1
} else {
std::thread::available_parallelism().map_or(1, |n| n.get())
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not make config.max_jobs a plain usize with a #[default = 0] and then first set it to 1 if config.fix || config.recursive, then if it is still zero, set it to available_parallelism if available, 1 otherwise?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed. Done.

@samueltardieu samueltardieu force-pushed the multithreading-lintcheck branch from d33e140 to a701af4 Compare March 8, 2023 21:30
@llogiq
Copy link
Contributor

llogiq commented Mar 9, 2023

Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 9, 2023

📌 Commit a701af4 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 9, 2023

⌛ Testing commit a701af4 with merge 48113d5...

bors added a commit that referenced this pull request Mar 9, 2023
lintcheck: use multithreading unless --fix or --recursive is used

Use multithreading unless there is a reason not to.
@bors
Copy link
Contributor

bors commented Mar 9, 2023

💔 Test failed - checks-action_test

@samueltardieu
Copy link
Member Author

@llogiq: I have added the missing "changelog: none" in the PR description

@llogiq
Copy link
Contributor

llogiq commented Mar 9, 2023

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 9, 2023

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.

@bors
Copy link
Contributor

bors commented Mar 9, 2023

📌 Commit a701af4 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 9, 2023

⌛ Testing commit a701af4 with merge 82e60d7...

bors added a commit that referenced this pull request Mar 9, 2023
lintcheck: use multithreading unless --fix or --recursive is used

Use multithreading unless there is a reason not to.

changelog: none
@bors
Copy link
Contributor

bors commented Mar 9, 2023

💔 Test failed - checks-action_test

@llogiq
Copy link
Contributor

llogiq commented Mar 9, 2023

Seems like a transient error

@bors retry

@bors
Copy link
Contributor

bors commented Mar 9, 2023

⌛ Testing commit a701af4 with merge a45f712...

@bors
Copy link
Contributor

bors commented Mar 9, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing a45f712 to master...

@bors bors merged commit a45f712 into rust-lang:master Mar 9, 2023
@samueltardieu samueltardieu deleted the multithreading-lintcheck branch March 24, 2023 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants