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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read command line options from ./.rubocop file and RUBOCOP_OPTS environment variable #3003

Merged
merged 1 commit into from
Apr 13, 2016

Conversation

bolshakov
Copy link
Contributor

This may be useful when running rubocop using another tool, for example pronto or on CI server.

Place one option per line:

$ cat .rubocop
--no-color
--fail-fast

Configuration options are loaded from .rubocop, command line switches, and the RUBOCOP_OPTS environment variable (listed in
lowest to highest precedence; for example, an option in .rubocop can be overridden by an option in RUBOCOP_OPTS).

I borrowed idea from the similar rspec feature.

@bolshakov bolshakov force-pushed the feature/read-options-from-files branch 2 times, most recently from f05045d to f8f3de7 Compare April 4, 2016 21:06
@bolshakov bolshakov changed the title Read command line options from ./.rubocop file. Read command line options from ./.rubocop file and RUBOCOP_OPTS environment variable Apr 4, 2016
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 5, 2016

Thanks for working on this. We were discussing such a feature a while back, but didn't introduce it as we thought it might be a bit confusing to have an extra config file. @jonas054 @alexdowad What do you think about this?

P.S. Everyone else is welcome to share their opinion as well - useful or confusing?

@alexdowad
Copy link
Contributor

I like it.

@bolshakov
Copy link
Contributor Author

For me this is not confusing if it is done explicitly, like in Rspec.

But it may be confusing when it's done implicitly like in bundler (running bundler with some options may produce hidden bundle config file.

I propose to make it explicitly, so you can fully control command line options.

@bolshakov bolshakov force-pushed the feature/read-options-from-files branch from bd267cd to db199c4 Compare April 5, 2016 06:36
@@ -200,6 +200,9 @@ Command flag | Description
`-s/--stdin` | Pipe source from STDIN. This is useful for editor integration.
`--[no-]color` | Force color output on or off.

Configuration options are loaded from `.rubocop`, command line switches, and the `RUBOCOP_OPTS` environment variable (listed in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Configuration or Command-line? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Configuration options may be taken from .rubocop file, command line options or environment variable.

How can I rephrase it to be more clear?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd use a wording like "Default command-line options are loaded from .rubocop and RUBOCOP_OPTS and are combined with command-line options that are explicitly passed to rubocop.". You should probably mention that explicitly passed params are going to have higher precedence.

@bolshakov bolshakov force-pushed the feature/read-options-from-files branch from db199c4 to a533204 Compare April 6, 2016 15:36

subject { options.parse(command_line_options).first }

it 'ENV options have priority over command line options' do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nothing should have a higher priority than command line options IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It breaks the whole idea. You can put .rubocop file in the repository, so every one in the team will get reasonable set of default options. If one need additional options, she can pass command line options and override .rubocop settings.

Another example, I have a Makefile for running various tasks in docker. So I can run specs, rubocop, etc. in isolated and reproducible environment. Its very helpful to pass special options in some environments.

When I run RSpec on CI server, I prefer to use JUnit formatter and redirect output to file, so CI server can read this file and generate pretty output. This is done by configuring CI server, to set environment variable SPEC_OPTS="--format RspecJunitFormatter --out rspec.xml".

Thus, I can decouple my build script from knowledge about specific CI servers.

So, the purpose of .rubocop file is to provide good defaults, but the purpose of RUBOCOP_OPTS is to externally override options.

For instance, .rubocop file may contains -D option, and RUBOCOP_OPTS may be set to --require rubocop/formatter/checkstyle_formatter --format RuboCop::Formatter::CheckstyleFormatter --no-color -out rubocop.xml

So I think that order of precedence should:

1.RUBOCOP_OPTS (Highest)
2. Command line options
3. .rubocop file (lowest)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see your point, but this seems super confusing to me and I definitely don't like it. Is anything besides rspec behaving like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did some research, and have not found any other examples. I also found a commit that adds such behaviour to RSpec. And the author just says "I suppose both ways are reasonable. SPEC_OPTS priority is better for patching default rspec behaviour."

It doesn't look like this order of precedence is a common practice. So I'd be happy to implement parsing options from environment variables in any order. Having this option is much more flexible when not having it at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

"The Art of UNIX Programming" by Raymond recommends:

CLI switches and arguments > Environment variables > Run-control files in home directory > Configuration files under /etc.

@bolshakov bolshakov force-pushed the feature/read-options-from-files branch from 8fb8a79 to 20c82df Compare April 11, 2016 08:32
@bolshakov
Copy link
Contributor Author

I updated PR to change order of precedence to the following:

  1. Explicit command-line options
  2. Options from RUBOCOP_OPTS environment variable
  3. Options from .rubocop file.

I also squashed commits into one and rebased branch on top of master.

…ariable.

This may be useful when running rubocop using another tool, for example [pronto](https://github.com/mmozuras/pronto).

Default command-line options are loaded from `.rubocop` and `RUBOCOP_OPTS` and are combined with command-line options that are explicitly passed to `rubocop`.
Thus, the options have the following order of precedence (from highest to lowest):

1. Explicit command-line options
2. Options from `RUBOCOP_OPTS` environment variable
3. Options from `.rubocop` file.
@bolshakov bolshakov force-pushed the feature/read-options-from-files branch from 20c82df to d6eed8f Compare April 11, 2016 19:45
@bbatsov bbatsov merged commit 092cb8a into rubocop:master Apr 13, 2016
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 13, 2016

👍

Neodelf pushed a commit to Neodelf/rubocop that referenced this pull request Oct 15, 2016
…ariable. (rubocop#3003)

This may be useful when running rubocop using another tool, for example [pronto](https://github.com/mmozuras/pronto).

Default command-line options are loaded from `.rubocop` and `RUBOCOP_OPTS` and are combined with command-line options that are explicitly passed to `rubocop`.
Thus, the options have the following order of precedence (from highest to lowest):

1. Explicit command-line options
2. Options from `RUBOCOP_OPTS` environment variable
3. Options from `.rubocop` file.
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

Successfully merging this pull request may close these issues.

3 participants