--no-parallel (-P/--parallel can not be combined with --auto-correct)

[kwerle]

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:

• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Wrote [good commit messages][1].
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• Added an entry (file) to the changelog folder named {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

[dvandersluis]

Looks like there was a Dockerfile accidentally included in the PR, can you remove it please?

[dvandersluis]

@kwerle thanks for the PR! I left you some small comments.

[rrosenblum]

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 --no-parallel always overrides --parallel? What if someone wants to do the opposite? Not that someone should, but what if they set --no-parallel in the config file and need to override it?

[dvandersluis]

@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.

I get a bit concerned with some flags overriding others. Would this be --no-parallel always overrides --parallel? What if someone wants to do the opposite? Not that someone should, but what if they set --no-parallel in the config file and need to override it?

Command line switches always have precedence over the .rubocop file.

[rrosenblum]

Command line switches always have precedence over the .rubocop file.

Awesome, this is already handled!

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

That makes sense. I sometimes forget that we constantly run into things where someone has a use case for the opposite of that thing

[kwerle]

Looks like there was a Dockerfile accidentally included in the PR, can you remove it please?

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.

[kwerle]

Concrete example for docker:

Some of the rake default tests fail in the container and I have no idea why (well, other than what it says). But it'd be really really nice if the container was able to run rake default cleanly. I'm guessing it's down to some environment variable or something.

Failure/Error: require 'webmock/rspec'

LoadError:
  cannot load such file -- webmock/rspec
# ./spec/spec_helper.rb:10:in `require'
# ./spec/spec_helper.rb:10:in `<top (required)>'
# tasks/spec_runner.rake:106:in `all_suite_files'
# /usr/local/bundle/gems/test-queue-0.4.2/lib/test_queue/runner.rb:57:in `initialize'
# tasks/spec_runner.rake:63:in `initialize'
# tasks/spec_runner.rake:30:in `new'
# tasks/spec_runner.rake:30:in `block in run_specs'
# tasks/spec_runner.rake:44:in `with_encoding'
# tasks/spec_runner.rake:28:in `run_specs'
# tasks/spec_runner.rake:116:in `block in <top (required)>'
Starting test-queue master (/tmp/test_queue_35_13540.sock)

[dvandersluis]

I'm not sure why the tests are failing for you (maybe try bundle exec rake default?) but if you want to add a dockerfile please do that as its own PR so that we can discuss.

Thanks for the changes! Your changelog file is not named properly -- it needs to start with change_ in this case so that the changelog script knows what section to put it in. You can also run rake changelog:change to generate a new file (but you'd have to manually delete your existing one).

[koic]

That's just my two cents. This alignment change looks unnecessary.

[koic]

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.

[marcandre]

Could we not have both options be compatible?

[kwerle]

I think that's it for the PR Feedback.

Thanks!

[kwerle]

Hmmm. Why are the rest of the tests not runnin?

[bbatsov]

Guess the build stuck or something. The change seems simple enough, so I'll just merge it as is.