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

Namespaces for cop names #1097

Closed
jonas054 opened this issue May 18, 2014 · 7 comments
Closed

Namespaces for cop names #1097

jonas054 opened this issue May 18, 2014 · 7 comments

Comments

@jonas054
Copy link
Collaborator

Based on the discussion in rubocop/rubocop-rspec#3

I'm working on a solution for namespaces and would like to discuss it. These are the ideas:

  1. Cop names can be prefixed with a namespace in lower case, so Encoding becomes style/Encoding, etc. This is done in .yml configuration files, in arguments to the --only option, and in inline comments, e.g., #rubocop:disable style/ClassLength.
  2. Output from --display-cop-names and --show-cops will include the namespace qualifiers.
  3. The namespace qualifiers are optional. There's a namespace lookup that will find the right namespace if none was given. If the cop name is ambiguous, execution will stop with an error message.
  4. No warning will be printed for missing namespace when a single namespace is found for the cop name.
@bbatsov
Copy link
Collaborator

bbatsov commented May 18, 2014

All points sound good to me. I also wonder if we shouldn't make it possible to use snake_case for cop names and config options, as I don't think the our pervasive use of CamelCase (or rather PascalCase) is particularly popular (or common) in the Ruby community.

@geniou
Copy link
Contributor

geniou commented May 18, 2014

Sounds good to me, just two things:

I think it would be good to be consistent by the writing of the namespace and the cop name, so both should be CamelCase, or both snake_case - in my opinion.

Also I think the new namespace should be added to the generated todo file.

@yujinakayama
Copy link
Collaborator

I agree with @geniou.

@bbatsov
Copy link
Collaborator

bbatsov commented May 18, 2014

My personal preference would be to use snake_case. The only reason we started with CamelCase names in the beginning was it was easier to just use the class names directly in the config. @jonas054 would correct me if I'm missing something. :-) Of course, this would be a serious change, so if we go this way we should keep supporting old naming scheme at least for one release.

@nevir
Copy link
Contributor

nevir commented May 18, 2014

+1 to consistent snake_case OR CamelCaps usage.

IMO, in the YAML file, the namespaces should be actual YAML keys: (organization, DRY). Downside being that you can no longer just append cops to the bottom of the file.

Also, it might be worth supporting both snake_case, and CamelCaps:

  • snake_case is great for human-facing configuration (and just general Ruby consistency)
  • CamelCaps is great for tools/scripts (plus, Rubocop owns the snake_case <-> CamelCaps encoding that way)
  • Or at least exposing snake/camel conversion utils as part of the public surface area

@jonas054
Copy link
Collaborator Author

@geniou Namespaces will be added to .rubocop_todo.yml. I forgot to mention that.

@nevir I'm not sold on the idea of having namespace keys with cop name keys under it. We would lose the uniformity of name references. Being able to put style/LineLength in .rubocop.yml, after --only, and after # rubocop:disable is a great advantage. You're right that it would be more dry, though.

About case... I think I was influenced by our internal use of cop types :lint and :rails, and perhaps also by conventions from C++, when I chose to have lower case namespaces with cop names still in CamelCase.

I'll change the namespaces to CamelCase, but I really don't feel like changing the cop names at this point, or supporting two naming styles. The gain would be small IMO.

@bbatsov
Copy link
Collaborator

bbatsov commented May 20, 2014

Let's go with @jonas054's idea for now. Btw, I think that the namespace can also be a cop key.

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

No branches or pull requests

5 participants