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

[#3246] Add cops list #3538

Merged
merged 1 commit into from Oct 17, 2016

Conversation

Projects
None yet
2 participants
@sihu
Contributor

sihu commented Sep 27, 2016

This is a partial fix of #3246. It includes a list with all available cops, sorted alphabetically with a short description and a link to the styleguide.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Sep 27, 2016

Collaborator

I'd suggest pulling from the source files of the cops their descriptions together with the code snippets illustrating how the cops work. Otherwise it'd be hard for people to figure out what exactly a cop is checking for.

Collaborator

bbatsov commented Sep 27, 2016

I'd suggest pulling from the source files of the cops their descriptions together with the code snippets illustrating how the cops work. Otherwise it'd be hard for people to figure out what exactly a cop is checking for.

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Sep 27, 2016

Contributor

👍 like that suggestion. My idea would be to summarize descriptions and snippets, so you see directly what the bot is doing, but if you want to know more, you can follow the link to the styleguide? Or do you have something different in mind?

Contributor

sihu commented Sep 27, 2016

👍 like that suggestion. My idea would be to summarize descriptions and snippets, so you see directly what the bot is doing, but if you want to know more, you can follow the link to the styleguide? Or do you have something different in mind?

Show outdated Hide outdated manual/cops.md Outdated
@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Sep 27, 2016

Collaborator

Probably we should breakdown the cop listings into several files for the various cop types. Also cop listings should say whether a cop is enabled/disabled by default and list config options if any. I'm guessing that after the description of each cop and the code snippets we could have some standard table listing the important attributes of the cop. Adding this information would be a tremendous improvement for all RuboCop users.

Collaborator

bbatsov commented Sep 27, 2016

Probably we should breakdown the cop listings into several files for the various cop types. Also cop listings should say whether a cop is enabled/disabled by default and list config options if any. I'm guessing that after the description of each cop and the code snippets we could have some standard table listing the important attributes of the cop. Adding this information would be a tremendous improvement for all RuboCop users.

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Sep 27, 2016

Contributor

@bbatsov so the output would be like that for each rule. Do you suggest any changes?

Style/VariableName:

This cop makes sure that all variables use the configured style,
snake_case or camelCase, for their names.

Example:

If the EnforcedStyle is snake_case it will offend all occurences of local variables with camelCase:

my_local = 1 #good
myLocal = 1  #bad

If the EnforcedStyle is camelCase it will offend all occurences of local variables with snake_case:

myLocal = 1  #good
my_local = 1 #bad

Default Settings:

Enabled EnforcedStyle SupportedStyles
true snake_case snake_case, camelCase
Contributor

sihu commented Sep 27, 2016

@bbatsov so the output would be like that for each rule. Do you suggest any changes?

Style/VariableName:

This cop makes sure that all variables use the configured style,
snake_case or camelCase, for their names.

Example:

If the EnforcedStyle is snake_case it will offend all occurences of local variables with camelCase:

my_local = 1 #good
myLocal = 1  #bad

If the EnforcedStyle is camelCase it will offend all occurences of local variables with snake_case:

myLocal = 1  #good
my_local = 1 #bad

Default Settings:

Enabled EnforcedStyle SupportedStyles
true snake_case snake_case, camelCase
@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Sep 27, 2016

Collaborator

@bbatsov so the output would be like that for each rule. Do you suggest any changes?

Support for auto-correction and whether there's auto-correction should be in the table as well.
Some cops might have other useful settings worth listing - e.g. default length, indentation, etc.
Might be better to make the table heading vertical, instead of horizontal. And for the comments add a space after #, otherwise RuboCop won't be happy. :-)

Overall I quite like this updated format.

Collaborator

bbatsov commented Sep 27, 2016

@bbatsov so the output would be like that for each rule. Do you suggest any changes?

Support for auto-correction and whether there's auto-correction should be in the table as well.
Some cops might have other useful settings worth listing - e.g. default length, indentation, etc.
Might be better to make the table heading vertical, instead of horizontal. And for the comments add a space after #, otherwise RuboCop won't be happy. :-)

Overall I quite like this updated format.

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Sep 27, 2016

Contributor

table with vertical heading in markdown is a pain, but i'm working on a solution 👍

Contributor

sihu commented Sep 27, 2016

table with vertical heading in markdown is a pain, but i'm working on a solution 👍

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Sep 27, 2016

Contributor

@bbatsov i have a next suggestion (i had some fights with myself because there is a difference between properties for each rule and attributes. so i split them up. What to you think about that?

Style/VariableName:

This cop makes sure that all variables use the configured style, snake_case or camelCase, for their names.

Properties:

  • Enabled by default: true
  • Supports autocorrection: true

Example:

If the EnforcedStyle is snake_case it will offend all occurences of local variables with camelCase:

my_local = 1 # good
myLocal = 1  # bad

If the EnforcedStyle is camelCase it will offend all occurences of local variables with snake_case:

myLocal = 1  # good
my_local = 1 # bad

Important attributes:

Attribute Value
Default EnforcedStyle camelCase
SupportedStyles snake_case, camelCase
Contributor

sihu commented Sep 27, 2016

@bbatsov i have a next suggestion (i had some fights with myself because there is a difference between properties for each rule and attributes. so i split them up. What to you think about that?

Style/VariableName:

This cop makes sure that all variables use the configured style, snake_case or camelCase, for their names.

Properties:

  • Enabled by default: true
  • Supports autocorrection: true

Example:

If the EnforcedStyle is snake_case it will offend all occurences of local variables with camelCase:

my_local = 1 # good
myLocal = 1  # bad

If the EnforcedStyle is camelCase it will offend all occurences of local variables with snake_case:

myLocal = 1  # good
my_local = 1 # bad

Important attributes:

Attribute Value
Default EnforcedStyle camelCase
SupportedStyles snake_case, camelCase
@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Sep 28, 2016

Collaborator

Good enough, I guess. Although I'd use a table for the properties as well - looks neater to me.

Collaborator

bbatsov commented Sep 28, 2016

Good enough, I guess. Although I'd use a table for the properties as well - looks neater to me.

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 5, 2016

Collaborator

@sihu Any progress here? I'm wondering if we'll be able to add this update to the upcoming release?

Collaborator

bbatsov commented Oct 5, 2016

@sihu Any progress here? I'm wondering if we'll be able to add this update to the upcoming release?

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Oct 5, 2016

Contributor

@bbatsov yes, please give me some more time, i think i can finish this evening. would that be early enough?

Contributor

sihu commented Oct 5, 2016

@bbatsov yes, please give me some more time, i think i can finish this evening. would that be early enough?

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 5, 2016

Collaborator

Yeah, that'd be fine.

On Wednesday, 5 October 2016, Simon Huber notifications@github.com wrote:

@bbatsov https://github.com/bbatsov yes, please give me some more time,
i think i can finish this evening. would that be early enough?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3538 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGVyuVChyx1sKR0H0sN3GMDd7mjpN0Cks5qw1rjgaJpZM4KHm9c
.

Best Regards,
Bozhidar Batsov

http://www.batsov.com

Collaborator

bbatsov commented Oct 5, 2016

Yeah, that'd be fine.

On Wednesday, 5 October 2016, Simon Huber notifications@github.com wrote:

@bbatsov https://github.com/bbatsov yes, please give me some more time,
i think i can finish this evening. would that be early enough?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3538 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGVyuVChyx1sKR0H0sN3GMDd7mjpN0Cks5qw1rjgaJpZM4KHm9c
.

Best Regards,
Bozhidar Batsov

http://www.batsov.com

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Oct 6, 2016

Contributor

i had some issues by trying to fetch the data directly from the source files, since i thought, it would be cool to have that automated (for now as a straight-forward-solution, but could also be implemented properly as a rake task). also that we are able to change the layout quicker. i found a way with YARD now. Hope you have a little bit more time, so i can finish:

  • get data from files with YARD
  • get data which are printed out via console (attributes, etc) and merge them to the first data
  • apply template
require 'yard'
require 'rubocop'

module RuboCop
  cop_comments = Hash.new
  YARD::Registry.load!
  cops = Cop::Cop.all
  YARD::Registry.all.select { |o| !o.docstring.blank? }.map do |o|
    name = o.path.gsub(/^RuboCop::Cop::([^#]*).*/, '\1').to_s
    type = name.gsub(/^([^:]*)/, '\1').to_s
    only_name = name.gsub(/::(.*)/, '\1').to_s
    cop_comments[name] = { doc: o.docstring, type: type, only_name: only_name}
  end.compact

  # Template
  cop_comments.each do |key, value|
    puts "-------------"
    puts "Cop: #{key}"
    puts "Type: #{value[:only_name]}"
    puts "-------------"
    puts value[:doc]
  end
end
Contributor

sihu commented Oct 6, 2016

i had some issues by trying to fetch the data directly from the source files, since i thought, it would be cool to have that automated (for now as a straight-forward-solution, but could also be implemented properly as a rake task). also that we are able to change the layout quicker. i found a way with YARD now. Hope you have a little bit more time, so i can finish:

  • get data from files with YARD
  • get data which are printed out via console (attributes, etc) and merge them to the first data
  • apply template
require 'yard'
require 'rubocop'

module RuboCop
  cop_comments = Hash.new
  YARD::Registry.load!
  cops = Cop::Cop.all
  YARD::Registry.all.select { |o| !o.docstring.blank? }.map do |o|
    name = o.path.gsub(/^RuboCop::Cop::([^#]*).*/, '\1').to_s
    type = name.gsub(/^([^:]*)/, '\1').to_s
    only_name = name.gsub(/::(.*)/, '\1').to_s
    cop_comments[name] = { doc: o.docstring, type: type, only_name: only_name}
  end.compact

  # Template
  cop_comments.each do |key, value|
    puts "-------------"
    puts "Cop: #{key}"
    puts "Type: #{value[:only_name]}"
    puts "-------------"
    puts value[:doc]
  end
end
@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 6, 2016

Collaborator

OK, I'll wait with the release.

Collaborator

bbatsov commented Oct 6, 2016

OK, I'll wait with the release.

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Oct 6, 2016

Contributor

@bbatsov. Since i had other tasks today i could not manage to finish. Maybe you release and i "deliver" as soon as possible within the next days. Promise!

Contributor

sihu commented Oct 6, 2016

@bbatsov. Since i had other tasks today i could not manage to finish. Maybe you release and i "deliver" as soon as possible within the next days. Promise!

@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 7, 2016

Collaborator

@sihu Fine by me. No worries!

Collaborator

bbatsov commented Oct 7, 2016

@sihu Fine by me. No worries!

@bbatsov bbatsov referenced this pull request Oct 15, 2016

Closed

List of Cops #3626

@sihu sihu closed this Oct 15, 2016

@sihu sihu reopened this Oct 15, 2016

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Oct 15, 2016

Contributor

i will just cleanup this PR, afterwards i think, i'll be ready 👍

Contributor

sihu commented Oct 15, 2016

i will just cleanup this PR, afterwards i think, i'll be ready 👍

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Oct 15, 2016

Contributor

@bbatsov sorry for my delay. i had not really capacity the last week. now i hope i could help the community :)

Contributor

sihu commented Oct 15, 2016

@bbatsov sorry for my delay. i had not really capacity the last week. now i hope i could help the community :)

Show outdated Hide outdated CHANGELOG.md Outdated
Show outdated Hide outdated manual/cops.md Outdated
Show outdated Hide outdated manual/cops_lint.md Outdated
@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 16, 2016

Collaborator

Looks pretty good! I've added a couple of tiny remarks and when addressed we'll be good to go.

You should probably add an item in the PR Template about running the docs update task when adding a new cop or changing a cop's config.

Collaborator

bbatsov commented Oct 16, 2016

Looks pretty good! I've added a couple of tiny remarks and when addressed we'll be good to go.

You should probably add an item in the PR Template about running the docs update task when adding a new cop or changing a cop's config.

@sihu

This comment has been minimized.

Show comment
Hide comment
@sihu

sihu Oct 17, 2016

Contributor

@bbatsov thx for your review. i updated the things and also changed the table format from vertical to horizontal, as discussed in the beginning. Some last questions:

  1. Did you ment the remark about the update-task in the other rake task, which creates a new cop template or a real template
  2. are we able to see, how the Markdown will be shown on readthedocs? i hope it's equal to github, because i just tested it here.
Contributor

sihu commented Oct 17, 2016

@bbatsov thx for your review. i updated the things and also changed the table format from vertical to horizontal, as discussed in the beginning. Some last questions:

  1. Did you ment the remark about the update-task in the other rake task, which creates a new cop template or a real template
  2. are we able to see, how the Markdown will be shown on readthedocs? i hope it's equal to github, because i just tested it here.
@bbatsov

This comment has been minimized.

Show comment
Hide comment
@bbatsov

bbatsov Oct 17, 2016

Collaborator

Did you ment the remark about the update-task in the other rake task, which creates a new cop template or a real template

I meant just that we should add a note in the PR_TEMPLATE.md file. I can do this myself.

are we able to see, how the Markdown will be shown on readthedocs? i hope it's equal to github, because i just tested it here.

As mentioned in the manual, you can install mkdocs locally and just run it on your computers.

Collaborator

bbatsov commented Oct 17, 2016

Did you ment the remark about the update-task in the other rake task, which creates a new cop template or a real template

I meant just that we should add a note in the PR_TEMPLATE.md file. I can do this myself.

are we able to see, how the Markdown will be shown on readthedocs? i hope it's equal to github, because i just tested it here.

As mentioned in the manual, you can install mkdocs locally and just run it on your computers.

@bbatsov bbatsov merged commit d516771 into rubocop-hq:master Oct 17, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@sihu sihu deleted the renuo:add-cops-list branch Oct 18, 2016

@pocke pocke referenced this pull request Dec 22, 2016

Merged

Automatically generate table of content of cops in manual #3814

7 of 11 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment