voxpupuli / json-schema

Ruby JSON Schema Validator
MIT License
1.53k stars 242 forks source link

Added rubocop.yml and corrected simple style warnings #283

Closed iainbeeston closed 3 weeks ago

iainbeeston commented 8 years ago

I'd like to enable HoundCI for json-schema. It works using rubocop to warn of style violations on pull requests. Before we can do that, we need to establish a rubocop style definition for the project and correct existing style violations.

This PR adds rubocop and houndci config, and corrects inconsistencies in the code style used throughout the project. Where two different styles were in use (in different files) I've tried to use whichever style is most common throughout the codebase.

RST-J commented 8 years ago

I also vote for adding rubocop as style checker. We use that in our company, too. As a reference we use the ruby styleguide .

I haven't read the whole config yet but I remember that rubocop by default has very strict cops regarding method and class size and complexity. So what exactly do want to have as limits in this respect? I mean as a goal, by now we won't score any acceptable values I guess.

iainbeeston commented 8 years ago

The rubocop config I've added uses the default Hound CI as the base, and then I've customised it to the norms in this project.

All of the complexity and class/method length metrics are disabled at the moment. I've corrected every (reasonable) rubocop error, so that in this branch, there are no rubocop warnings at all.

RST-J commented 8 years ago

Ok, I'll try to find time to investigate the config some time soon - I think we'll go for it, the question is if everyone agrees about the defined style. The one thing that immediately came to my mind - no spaces in hash literals - is currently in my favour :grinning:

iainbeeston commented 8 years ago

With the style config, I've tried to make it enforce the most common style throughout the codebase. For example, we use double-quotes more than single, so that became the enforced style. So hopefully it's representative, and just makes things more consistent. However I'd be more than happy to adjust it to suit tastes, or make it more strict

RST-J commented 8 years ago

I don't intend to provoke hard discussions, probably its all fine already or there are only minor things, I don't know. But I think at least we four, i.e. also @hoxworth and @pd, should agree upon the config when we introduce it.

iainbeeston commented 8 years ago

Sure, that sounds sensible. I'm not in a rush.

RST-J commented 7 years ago

Do we want to go on with this? I'd say yes. The other two would have had enough time to react, I'd say.

bolshakov commented 3 weeks ago

It should be safe to close this PR, rubocop was added here https://github.com/voxpupuli/json-schema/pull/480