CircleCI-Public / ruby-orb

The `circleci/ruby` orb source code.
https://circleci.com/developer/orbs/orb/circleci/ruby
MIT License
26 stars 53 forks source link

Change parallel settings #144

Closed geeknees closed 2 months ago

geeknees commented 5 months ago

In the current state of Rubocop, parallel settings appear to be enabled by default. Therefore, this setting is also enabled by default.

https://docs.rubocop.org/rubocop/usage/basic_usage.html#command-line-flags

In the documentation, the default in orb is stated as false, but I think the actual behaviour is parallel operation. For users who explicitly stated false, this would change the behaviour. However, I would like the orbs default behaviour to be aligned with the default behaviour of Rubocop.

There is an idea to abolish <> and introduce <>, although this would force existing configuration changes when updating orb.

If this is better, please point this out.

marboledacci commented 2 months ago

This change works on newer versions of Rubocop, but the --no-parallel flag doesn't exist in older versions. Unfortunately I haven't been able to find the exact version where the parallel behavior changes, so until then we cannot do a good fix on this. I'll will let the PR open in case you want to add this correction.

geeknees commented 2 months ago

@marboledacci

Thank you for the review. It seems that the --no-parallel option was introduced in v1.13.0, and the default behavior was changed starting from v1.32.0.

https://github.com/rubocop/rubocop/blob/master/relnotes/v1.13.0.md https://github.com/rubocop/rubocop/blob/master/relnotes/v1.32.0.md

However, I am unsure how to best address this. Although the current default is documented as false, it is still executing in parallel, which contradicts the documentation. I would like to fix this.

Should I implement a version check within the shell script to switch behavior? (For versions prior to v1.13.0, we wouldn’t pass --no-parallel, as the default should already be --no-parallel in those versions.)

marboledacci commented 2 months ago

I think a version check is the way to go