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

Improve rubocop todo #693

Merged
merged 2 commits into from
Dec 22, 2013
Merged

Improve rubocop todo #693

merged 2 commits into from
Dec 22, 2013

Conversation

jonas054
Copy link
Collaborator

I've been thinking that we should be able to do better than just generating Enabled: false in rubocop-todo.yml. Here's an implementation that sets Max (for all cops that have a Max) and EnforcedStyle (for some that have it).

So I haven't gone all the way and implemented the functionality for all configurable cops. I wanted to see what people think of it first. It does require a little bit of code added to the cops.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling d6e302128d99aa624f340f6d78bbb8f087d3d2b6 on jonas054:improve_rubocop_todo into ab06b29 on bbatsov:master.

IndentationWidth::CORRECT_INDENTATION - expected_indent_offset
end

def other_style
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

complementing_style or alternative_style would be a better name.

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 22, 2013

I like the general direction of the change. I'd suggest that the logic selecting a style in many cops (usually a switch on config['EnforcedStyle'] be extracted as well to the enforced style module (this would require the supported values to be supplied in the config, but would simplify the code).

@jonas054
Copy link
Collaborator Author

Good comments. I've made separate commits that I'll squash when you think everything is OK.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.13%) when pulling 50d4d7c57b53c7b912929772fe3cfb986f9ac779 on jonas054:improve_rubocop_todo into ab06b29 on bbatsov:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.13%) when pulling 2382e322224e8b190105237f850b0215bd72fee6 on jonas054:improve_rubocop_todo into ab06b29 on bbatsov:master.

@jonas054
Copy link
Collaborator Author

OK now?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.11%) when pulling d0828e76b81152804c034de1980c59ce66b1adf3 on jonas054:improve_rubocop_todo into ab06b29 on bbatsov:master.

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 22, 2013

Yes. —
Sent from Mailbox for iPhone

On Sun, Dec 22, 2013 at 6:21 PM, Coveralls notifications@github.com
wrote:

Coverage Status

Coverage remained the same when pulling d0828e76b81152804c034de1980c59ce66b1adf3 on jonas054:improve_rubocop_todo into ab06b29 on bbatsov:master.

Reply to this email directly or view it on GitHub:
#693 (comment)

All the Max parameters and some EnforcedStyle parameters can now
be generated into rubocop-todo.yml. More can be added later.

These parameters are generated instead of Enabled:false, making the
available configuration options more visible.
@jonas054
Copy link
Collaborator Author

Fixed one more thing. There was a double definition of DEFAULT_CONFIG in the specs. And squashed.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.11%) when pulling 74a99ee on jonas054:improve_rubocop_todo into ab06b29 on bbatsov:master.

bbatsov added a commit that referenced this pull request Dec 22, 2013
@bbatsov bbatsov merged commit 7c259f5 into rubocop:master Dec 22, 2013
@jonas054 jonas054 deleted the improve_rubocop_todo branch January 19, 2014 21:24
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