saab-simc-admin / palletjack

Lightweight Inventory/Asset/Configuration Management Database
MIT License
5 stars 5 forks source link

RFE: Rubocop #106

Closed pingram3030 closed 7 years ago

pingram3030 commented 7 years ago

To ensure consistency of coding styles and ruby best practice, rubocop is a great tool. Its cops can be enabled/disabled and modified/tuned via a .rubocop.yml file in the root of the repo. Would you, the devs, be interested in integrating rubocop to assist with your testing? It can of course easily be integrated as a rake task.

pingram3030 commented 7 years ago

Here's my branch that creates the basics for copping palletjack. Overrides can either be put in to the main .rubocop.yml file, or in the code where required. I hope this is of use; rake passes all tests.

creideiki commented 7 years ago

Thanks for the code; review will be slow while I'm at LCA and the rest of the team are covering my duties back home, but I will look at it when I get back (unless someone else beats me to it).

creideiki commented 7 years ago

Having run RuboCop stand-alone, without looking at this code, it certainly looks like something we want to use.

pingram3030 commented 7 years ago

It can be used to auto-correct some code too, which is nice - rubocop -a. Working through the 'todo' list looks intimidating, but most of them are simple fixes. The biggest thing to do is for the core devs to decide on conventions and styles. Some of the cops support multiple styles which should help tailor things to your liking;

for example: Style/EmptyLinesAroundBlockBody supports the styles of empty_lines or no_empty_lines

Style/HashSyntax supports the styles of ruby19, hash_rockets, no_mixed_keys, ruby19_no_mixed_keys

Style/MethodName supports snake_case or camelCase

It would be my preference/suggestion to do minimal overrides, but you should choose what works best for other code you write; for continuities sake. Maybe quickly run through the 'important' ones, set them in .rubocop.yml and then I could assist in cleaning up what's left?

creideiki commented 7 years ago

We think that the best way forward would be to add RuboCop to the build chain now, and then address potentially contentious issues one by one later, to avoid the entire thing getting hung up on religious issues about tab width. As such, we'd be happy to accept a pull request containing, as separate commits:

  1. Enabling of RuboCop in the default Rake target, with all default settings.
  2. An updated README, with contributor guidelines stating that pull requests shall pass the RuboCop tests.
  3. The auto-generated .rubocop_todo.yml.
  4. An auto-correction of Style/StringLiterals, including removing it from .rubocop_todo.yml.
  5. An auto-correction of Style/SpaceInsideParens, including removing it from .rubocop_todo.yml.
  6. An auto-correction of Style/AlignHash, including removing it from .rubocop_todo.yml.
  7. Any other obviously correct auto-corrections you can find. Those three were the ones I ran manually just now.