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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

make --parallel and --auto-correct work together #10613

Closed
grosser opened this issue May 9, 2022 · 3 comments
Closed

make --parallel and --auto-correct work together #10613

grosser opened this issue May 9, 2022 · 3 comments

Comments

@grosser
Copy link
Contributor

grosser commented May 9, 2022

I can do it with xargs, so we can also do it internally 馃し
... maybe it needs to say "rules xyz are not enforced because they would rename files" or whatever the reason was to disable this, but for 99% case they should just work! :)

@jonas054
Copy link
Collaborator

jonas054 commented Jul 4, 2022

@marcandre asked about the same thing here but it was never answered.

I think the reason why we've disabled parallel execution for autocorrection is that it's not trivial to get it to work consistently. Just removing the lines

('-x/--fix-layout' if @options.key?(:fix_layout)),
('-a/--autocorrect' if @options.key?(:safe_autocorrect)),
('-A/--autocorrect-all' if @options.key?(:autocorrect_all)),
mostly does what we want, but the output from rubocop -a -P is always a long row of green dots. I.e., we don't see any indication of offenses that were found and corrected. So the normal algorithm for parallel execution, which is to warm up the cache in parallel and then do the inspection in a single thread, doesn't fulfill the needs of autocorrection.

jonas054 added a commit to jonas054/rubocop that referenced this issue Jul 9, 2022
Allow the `-P`/`--parallel` flag together with the autocorrection
flags `--[safe-]auto-correct`/`-a` `--auto-correct-all`/`-A` and
`--fix-layout`. Also make this the default, so that `--no-parallel`
has to be given for a single core run.

The algorithm for normal parallel inspection is followed, so that a
cache warm-up phase is run first. This phase only does caching of
inspection results. It doesn't update any files. Then it does a single
core run that takes advantage of the cached results and does the
corrections. The speed gain from this approach is probably smaller
than writing files in parallel, but it's a pretty small change that
still outputs correction results in the same way as before.

Also fix the documentation for `--parallel`, which did not mention
`--no-parallel`.
@grosser
Copy link
Contributor Author

grosser commented Jul 11, 2022

amazing! :D

@jonas054
Copy link
Collaborator

@grosser Thanks! I hope you read in the PR that it's a somewhat limited improvement that only guarantees that the cache is warm before starting to correct files. Sometimes that translates into a big gain in speed, sometimes it's a small one. 馃檪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants