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

Whitelist the Rakefile and Gemfile from name check #878

Closed
wants to merge 1 commit into from

Conversation

theckman
Copy link

When building a Ruby project it's very common that you'll have a Rakefile and a Gemfile. These files are normally capitalized, which violates the default Rubocop filename checks. This changeset adds a whitelist for base filenames that should NOT be included in the file_name checks.

This works by adding a constant to the FileName class, WHITELIST, which is an Array that contains the whitelisted basenames. Later on, it will only add an offense if the basename is not snake case, and the basename is not Rakefile or Gemfile.

I've updated the tests for this specific use case and have confirmed that all tests pass on my local system.

-Tim

When building a Ruby project it's very common that you'll have a Rakefile and a Gemfile. These files are normally capitalized, which violates the default Rubocop filename checks. This changeset adds a whitelist for base filenames that should *NOT* be included in the file_name checks.

This works by adding a constant to the FileName class, WHITELIST, which is an Array that contains the whitelisted basenames. Later on, it will only add an offense if the basename is not snake case, and the basename is not Rakefile or Gemfile.

I've updated the tests for this specific use case and have confirmed that all tests pass on my local system.

-Tim
@yujinakayama
Copy link
Collaborator

Could you explain your situation where RuboCop registers offenses for Rakefile and Gemfile in more detail?

Normally RuboCop inspect files with .rb extension or files with no extension and ruby shebang, so Rakefile and Gemfile won't be inspected unless you specify them in AllCops/Includes in your .rubocop.yml (though Rakefile is already in the default built-in configuration). And Filename cop accepts any filename if it's specified in AllCops/Includes.

@theckman
Copy link
Author

I found this behavior with the following configuration in my Rakefile caused my CI to fail the build:

Rubocop::RakeTask.new(:rubocop) do |t|
  t.patterns = %w{ Rakefile Gemfile rmuh.gemspec lib/**/*.rb spec/*_spec.rb }
  t.fail_on_error = true
end

I wanted to make sure my Rakefile and Gemfile both are within rubocop compliancy. If there is a better option that doesn't require this patch I'm open for suggestions.

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 14, 2014

@theckman You can either list the files in AllCops/Includes or in FileName/Exclude.

@yujinakayama
Copy link
Collaborator

@theckman I understood your case. I forgot the case where file paths are explicitly passed in CLI.

@bbatsov There's no FileName/Exclude configuration currently.

IMO, the whitelist should be configurable rather than fixed list.

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 14, 2014

@yujinakayama Every cop has Include and Exclude. :-) Those are global cop params like Enabled. I guess we should simply list all known problematic files in the default Exclude.

@yujinakayama
Copy link
Collaborator

@bbatsov Forgot about it. :) By the way, are these typos?

I guess we should simply list all known problematic files in the default Exclude.

👍

@bbatsov
Copy link
Collaborator

bbatsov commented Mar 14, 2014

Totally. I must have been copy-pasting an entry with a typo. :-) Should be more careful. I’ll fix both issues now.

Cheers,
Bozhidar

On Friday, March 14, 2014 at 6:45 PM, Yuji Nakayama wrote:

@bbatsov (https://github.com/bbatsov) Forgot about it. :) By the way, are these (https://github.com/bbatsov/rubocop/blob/522f1341ca836de6f39448cabb3495b5b406068b/config/default.yml#L423-L444) typos?

I guess we should simply list all known problematic files in the default Exclude.


Reply to this email directly or view it on GitHub (#878 (comment)).

@bbatsov bbatsov closed this in 6d18223 Mar 14, 2014
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