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

Add style cop that checks for inline comments #1062

Merged
merged 6 commits into from
May 30, 2014

Conversation

salbertson
Copy link
Contributor

  • Create custom matcher for testing offenses

@bbatsov
Copy link
Collaborator

bbatsov commented May 2, 2014

Looks good. Please, a changelog entry.

@geniou
Copy link
Contributor

geniou commented May 2, 2014

Usually the number of offenses, the line number, the message and the highlights are checked in the specs. I think this is here useful too. Just take a look in the newly added specs.

@bbatsov
Copy link
Collaborator

bbatsov commented May 8, 2014

@salbertson Ping :-)

@salbertson
Copy link
Contributor Author

@bbatsov, I'll make those changes soon. Thanks for checking in.

@salbertson
Copy link
Contributor Author

@geniou, would you link to a test that checks the things you mentioned? I looked at a couple recent Cop specs and didn't find any tests for line numbers, message or highlights.

@geniou
Copy link
Contributor

geniou commented May 21, 2014

@salbertson good question - I think @bbatsov has to decide. General it would be good if all (new) specs test these things, but I'm not sure if its wise / possible to enforce it.

@bbatsov
Copy link
Collaborator

bbatsov commented May 21, 2014

@salbertson Message checking makes sense only if the cop does dynamic message generation. Most cops should do highlights checking in the specs, but older don't as we hadn't originally devised this check.

Personally, I rarely check the line numbers of the offenses in the specs I write.

Anyways, you can have a look at the variable_name_spec.rb and the trivial_accessors_specs.rb for example code.

* Create custom matcher for testing offenses
@salbertson
Copy link
Contributor Author

@bbatsov Great, thanks.

@salbertson
Copy link
Contributor Author

@bbatsov @geniou Sorry this took a while. I'd appreciate another review when you have a chance.

I added a custom matcher to make testing for offenses and highlighted easier. Hopefully the new matcher will encourage others to test for highlights along with offenses.

@salbertson
Copy link
Contributor Author

I also added a matcher that to help test messages.

bbatsov added a commit that referenced this pull request May 30, 2014
Add style cop that checks for inline comments
@bbatsov bbatsov merged commit 59ace00 into rubocop:master May 30, 2014
@bbatsov
Copy link
Collaborator

bbatsov commented May 30, 2014

👍 The code looks good.

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