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

Do we really need Naming/InclusiveLanguage enabled by default? #9895

Closed
ShockwaveNN opened this issue Jun 29, 2021 · 4 comments
Closed

Do we really need Naming/InclusiveLanguage enabled by default? #9895

ShockwaveNN opened this issue Jun 29, 2021 · 4 comments

Comments

@ShockwaveNN
Copy link
Contributor

@ShockwaveNN ShockwaveNN commented Jun 29, 2021

I think this cop should be disable by default, because there is a lot of github repos that still using master as default branch and any code that works with those repo will be flagged

Actual behavior

  1. Run rubocop on this pseudo-code
# frozen_string_literal: true

def repo_fetcher(repo, branch)
  p "I fetch #{repo} from #{branch}"
end

repo_fetcher('github.com/org/repo-with-master', 'master')
  1. rubocop test.rb
Inspecting 1 file
C

Offenses:

test.rb:7:40: C: Naming/InclusiveLanguage: Consider replacing problematic term 'master' with 'main', 'primary', or 'leader'.
repo_fetcher('github.com/org/repo-with-master', 'master')
                                       ^^^^^^
test.rb:7:50: C: Naming/InclusiveLanguage: Consider replacing problematic term 'master' with 'main', 'primary', or 'leader'.
repo_fetcher('github.com/org/repo-with-master', 'master')

RuboCop version

$ [bundle exec] rubocop -V
1.18.0 (using Parser 3.0.1.1, rubocop-ast 1.7.0, running on ruby 2.7.3 x86_64-linux)
@ShockwaveNN
Copy link
Contributor Author

@ShockwaveNN ShockwaveNN commented Jun 29, 2021

Found real very simple code example:
https://github.com/ONLYOFFICE-QA/onlyoffice_documentserver_conversion_helper/blob/master/onlyoffice_documentserver_conversion_helper.gemspec#L21

This line in gemspec raise a hit

    'changelog_uri' => "#{s.homepage}/blob/master/CHANGELOG.md",

koic added a commit to koic/rubocop that referenced this issue Jun 29, 2021
Fixes rubocop#9895 and follow up rubocop#9893 (comment).

This PR disables `Naming/InclusiveLanguage` by default because
it has an unexpectedly impact for many users who give feedback.
@bbatsov
Copy link
Collaborator

@bbatsov bbatsov commented Jun 29, 2021

I agree that it might be too much for some cases. I was always concern about checking strings by default. Btw, the cop's configuration is super flexible - one can easily limit this only to identifiers for instance. Anyways, I'm definitely considering disabling the cop by default.

koic added a commit to koic/rubocop that referenced this issue Jun 29, 2021
… `FlaggedTerms` for `Naming/InclusiveLanguage`

Fixes rubocop#9895 and follow up rubocop#9893 (comment).

This PR sets `CheckStrings: false` and removes `master` from `FlaggedTerms` for `Naming/InclusiveLanguage`
because it has an unexpectedly impact for many users who give feedback.
koic added a commit to koic/rubocop that referenced this issue Jun 29, 2021
… `FlaggedTerms` for `Naming/InclusiveLanguage`

Fixes rubocop#9895 and follow up rubocop#9893 (comment).

This PR sets `CheckStrings: false` and removes `master` from `FlaggedTerms` for `Naming/InclusiveLanguage`
because it has an unexpectedly impact for many users who give feedback.
bbatsov added a commit that referenced this issue Jun 29, 2021
…edTerms` for `Naming/InclusiveLanguage`

Fixes #9895 and follow up #9893 (comment).

This PR sets `CheckStrings: false` and removes `master` from `FlaggedTerms` for `Naming/InclusiveLanguage`
because it has an unexpectedly impact for many users who give feedback.
@bbatsov
Copy link
Collaborator

@bbatsov bbatsov commented Jun 30, 2021

We've cut a new release with a more permissive configuration for the cop. It shouldn't be noisy for anyone at this point.

@ShockwaveNN
Copy link
Contributor Author

@ShockwaveNN ShockwaveNN commented Jun 30, 2021

@bbatsov Thanks for very fast reaction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants