-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add a CopCount formatter #439
Conversation
Nice idea, but I have some feedback:
@yujinakayama @jonas054 @edzhelyov Would you like to add something more? |
|
Looks good. Thanks! |
Sorry for this late reply, but I still want to comment.
|
offence_count = total_offence_count(cop_offences) | ||
cop_offences.each do |cop_name, count| | ||
count_string = "(#{count.to_s})" | ||
output.puts "#{count_string.ljust(offence_count + 4)}#{cop_name}\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't be ljust(offence_count + 4)
. If you really want to base the justification on the total count, it should be ljust(offence_count.to_s.length + 4)
or something to that effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally correct. No idea how this slipped through.
I really like OffenceCount, in fact, I had the same idea (Offences over Cops) today after rethink off @bbatsov's comments and was in the process of renaming anyway! |
Well, the new version is still not out, so feel free to rename the formatter and address the rest of @jonas054's comments in a separate PR. |
Allows you to see a list of cops with offences, ordered by offence count.
e.g.