Closed Cantido closed 9 years ago
The failing builds are because of style reasons now, correct?
Yes
On Wed, Mar 25, 2015, 8:01 AM Andrew Brown notifications@github.com wrote:
The failing builds are because of style reasons now, correct?
— Reply to this email directly or view it on GitHub https://github.com/drusepth/Indent/pull/423#issuecomment-86014838.
I'm working through the CharactersController now, this is actually kind of difficult. Rubocop expects methods to be quite short. Also, the complexity requirement leaves very little room in the Controller's methods for any work besides the format calls. I may tweak the Rubocop settings to allow these methods to be a little bigger.
So Rubocop actually has an auto-correct feature, and it cleaned up a ton of the tedium! We're down from a few thousand violations to a few hundred!
There's also a lot of i18n happening, since that's a great way to make lines shorter :+1:
WOW so I just refactored the GeneratorController
. It was a lot bigger change than I was expecting to make, however I do think this was a really great refactoring! I split up the GeneratorController
into an EquipmentGeneratorController
, a LocationsGeneratorController
, and a CharactersGeneratorController
. Of course that means I had to modify routes.rb
to point to the new classes as well. I am really happy with this commit in particular. RuboCop really forces you to clean things up.
So I use Brackets as my IDE, and I was frustrated at how the RuboCop plugin was broke as hell, so I fixed it: brackets-rubocop. This will make the job a little easier.
Only 81 offenses in 16 files remaining! Here's a summary of the changes left to make:
44 Metrics/LineLength
15 Metrics/AbcSize
11 Metrics/MethodLength
4 Style/Documentation
4 Style/IfUnlessModifier
2 Style/CommentAnnotation
1 Lint/UselessAssignment
--
81 Total
Done! You can have the honor of merging it, once you have a look and you're satisfied with the changes.
Wow, this looks really, really good. Aside from that one string comment it looked really solid, and the refactoring/cleanup is something the code's needed for a really long time. Thanks for this!
My pleasure! It was a real struggle in a few places, and I did end up having to disable RuboCop in a few places, but the tight restrictions really force you to refactor like crazy. I also really like how it forces you to restructure some if statements to use unless
, and to return
early instead of putting a whole function in an if
block.
The initial commit, d03fa2d, adds a rubocop scan to travis, so the build will fail until we clean this up.
This will fix #368 upon merge.