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
--no-parallel (-P/--parallel can not be combined with --auto-correct) #9640
Conversation
b99e044
to
d60738d
Compare
Looks like there was a Dockerfile accidentally included in the PR, can you remove it please? |
@kwerle thanks for the PR! I left you some small comments. |
The rake task will automatically delete the parallel flag when running auto-correct. I wounder if the solution to this would be to internalize that behavior rather than introducing more flags. Maybe we can automatically ignore parallel in the config when trying to auto-correct? I have not dug into that area of the code lately so I'm not sure how difficult that would be to handle. I get a bit concerned with some flags overriding others. Would this be |
@rrosenblum I had the same thought -- if parallel is specified along with one of the other flags that conflict with it, it'd be good to just disable parallel and show a warning. At the same time, I think it's useful to have a way to disable parallelization if desired, separate from handling the conflicts so I was thinking of introducing that in a separate PR.
Command line switches always have precedence over the |
Awesome, this is already handled!
That makes sense. I sometimes forget that we constantly run into things where someone has a use case for the opposite of that thing |
I will if you insist - but I find it super handy. I don't have [any reasonable version of] ruby installed. In addition, it means that anyone with docker knows exactly what everyone else is dealing with. So I will if you like. But otherwise I would change the command to rake default and it would be super obvious which tests are failing consistently to everyone. |
Concrete example for docker: Some of the
|
I'm not sure why the tests are failing for you (maybe try Thanks for the changes! Your changelog file is not named properly -- it needs to start with |
lib/rubocop/options.rb
Outdated
@@ -365,7 +365,7 @@ def validate_parallel_with_combo_option | |||
auto_gen_config: '-P/--parallel uses caching to speed up execution, ' \ | |||
'while --auto-gen-config needs a non-cached run, ' \ | |||
'so they cannot be combined.', | |||
fail_fast: '-P/--parallel cannot be combined with -F/--fail-fast.', | |||
fail_fast: '-P/--parallel cannot be combined with -F/--fail-fast.', |
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 just my two cents. This alignment change looks unnecessary.
Thank you for explanation for Dockerfile in the PR. But AFAIK, unlike application development, Dockerfile is generally not used and provided for gem development. So, please remove the Dockerfile and squash your commits into one. |
Could we not have both options be compatible? |
I think that's it for the PR Feedback. Thanks! |
Consistent documentation. Co-authored-by: Daniel Vandersluis <daniel.vandersluis@gmail.com>
* Remove docker * Spacing
3a2c248
to
3b7c027
Compare
Hmmm. Why are the rest of the tests not runnin? |
Guess the build stuck or something. The change seems simple enough, so I'll just merge it as is. |
Fix #7544
We just discovered --parallel and added it to our .rubocop file. Turns out you can't use it with -a.
We would like to leave --parallel in our .rubocop file but would occasionally like to use -a. So we would like a --no-parallel flag to override our default - so we can do
rubocop --no-parallel -a
Replace this text with a summary of the changes in your PR.
The more detailed you are, the better.
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.Note that master failed
bundle exec rake default