Bump rubocop to 0.37.2 #48

jpignata · 2016-01-31T17:45:18Z

Bump to Rubocop 0.37.1. See: https://github.com/bbatsov/rubocop/blob/master/CHANGELOG.md
Introduce SourceFile
Rename ViolationDecorator to Issue
Move presentation concerns into Issue
Refine remediation point names to better match what they represent
Move from minitest to rspec
Begin work to improve coverage

@codeclimate/review

See: https://github.com/bbatsov/rubocop/blob/master/CHANGELOG.md#0360-14012016

jpignata · 2016-01-31T17:52:17Z

Broke the camel's back in CC::Engine::Rubocop, apparently.

wfleming · 2016-01-31T20:28:16Z

I'm ok with ignoring that line count if you want to, but I also see a fairly easy extraction: this class right now covers all of:

Walking include_paths for files to analyze
Getting analysis results on those file from Rubocop
Formatting said results as Issue JSON to emit on stdout.

Item 3 by itself is ~35 lines, and feels like a distinct responsibility that we already organize as its own class in other engines. Personally, that's where I'd make the change.

jpignata · 2016-01-31T20:40:28Z

@wfleming Nah, we should fix it. It does way too much. I'm extracting things from it now but in doing so I'm also seeing language that doesn't align with our domain very well and a lack of coverage around fiddly bits like remediation point generation. I want to spend a bit more time tackling those issues before requesting re-review.

jpignata · 2016-02-01T03:33:47Z

@wfleming Ready for 👀 when you have a sec.

jpignata · 2016-02-01T03:48:37Z

Interestingly they've deprecated one of the cops and if referenced in the rubocop.yml, the anaysis will fial:

Running rubocop: Done!
error: (CC::Analyzer::Engine::EngineFailure) engine rubocop failed with status 1 and stderr 
/usr/lib/ruby/gems/2.2.0/gems/rubocop-0.36.0/lib/rubocop/config.rb:250:in `reject_obsolete_cops': The `Style/TrailingComma` cop no longer exists. Please use `Style/TrailingCommaInLiteral` and/or `Style/TrailingCommaInArguments` instead. (RuboCop::ValidationError)
(configuration found in /code/.rubocop.yml)

Our default configuration references Style/TrailingComma.

jpignata · 2016-02-01T04:00:21Z

The Code Climate issues are due to changing cop names (Style/MultilineOperationIndentation and Style/TrailingComma).

Code Climate triggered a warning that this class was too large and looking closely it seems to have several different responsibilities mixed together. At the same time, a lot of the language used in this engine doesn't match the language of how we talk about our domain. These changes aim to break down the Rubocop class into collaborators and to refine the language to bring its concerns closer to our domain language. * Introduce SourceFile * Rename ViolationDecorator to Issue * Move presentation concerns into Issue * Refine remediation point names to better match what they represent * Begin to improve coverage on remediation point generation

wfleming · 2016-02-01T16:12:10Z

This looks good to me. If we can get the updated default config out, I'd say ship it.

jpignata · 2016-02-01T16:37:44Z

Going to let this sit for a bit until the larger CLI change is out.

On Mon, Feb 1, 2016 at 11:12 AM, Will Fleming [email protected]
wrote:

This looks good to me. If we can get the updated default config out, I'd
say ship it.

—
Reply to this email directly or view it on GitHub
#48 (comment)
.

ABaldwinHunter · 2016-02-02T18:55:59Z

Hm, surprised by CC issues; I thought we prefer including a comma after last item in hash.

wfleming · 2016-02-02T19:00:07Z

@ABaldwinHunter I believe it's because JP updated the config for the new version, but of course that new version isn't actually running against the repo yet. The new version changed several rule names.

Instead of raising on the inclusion of an obsolete cop, write an error to STDERR. This is slightly more invisible than failing a build on an obsolete cop, but preferable as a user won't be blocked when we bump RuboCop on our engine. We'll write a changelog post advising users of the necessary changes to make and deprecate this patch in the coming weeks.

jpignata · 2016-02-11T15:07:24Z

This is ready for another look, @codeclimate/review.

gdiggs · 2016-02-11T15:14:47Z

LGTM

wfleming · 2016-02-11T18:29:31Z

From a quick reading of recent changes in whitequark/parser, and confirming that we are on the most recent version of that gem now, I think this might give us support for Ruby 2.3 syntax. Worth testing a bit more specifically to see if that's worth promoting on release.

jpignata · 2016-02-11T19:05:20Z

It does give us support for Ruby 2.3 syntax. I'll merge this once I have a
changelog draft.

On Thu, Feb 11, 2016 at 1:29 PM, Will Fleming [email protected]
wrote:

From a quick reading of recent changes in whitequark/parser, and
confirming that we are on the most recent version of that gem now, I
think this might give us support for Ruby 2.3 syntax. Worth testing a
bit more specifically to see if that's worth promoting on release.

—
Reply to this email directly or view it on GitHub
#48 (comment)
.

wfleming · 2016-02-11T19:06:18Z

🎆

ABaldwinHunter · 2016-02-11T19:20:32Z

@jpignata nice work! reminder to update doc in addition to changelog once all set.

Bump rubocop to 0.37.2

rheaton · 2016-02-12T21:22:39Z

Was this just deployed? Rubocop upgrade breaks older rubocop.yml (we see analysis errors now)

rheaton · 2016-02-12T21:22:56Z

P.S. hi JP. 👋

jpignata · 2016-02-12T21:25:23Z

Hey, @rheaton! Yes, we just deployed this earlier this afternoon. Some configuration syntax moved around between the RuboCop versions which require some tweaks to your config. If you check the build itself, the STDERR output should detail the changes necessary. Can you let me know if you see this output and if it's helpful?

rheaton · 2016-02-12T21:27:23Z

Yes, we see it! It's slow going through since it's one error at a time. Any suggestions?

(PS My teammate @MisterDeejay is working on this)

jpignata · 2016-02-12T21:29:54Z

It's probably giving you separate tranches of output for each of RuboCop's validations. I'd either use the Code Climate CLI or install RuboCop 0.37.2 locally and tweak your configuration there before re-pushing.

rheaton · 2016-02-12T21:34:33Z

@jpignata Thanks!

jpignata · 2016-02-12T21:35:02Z

@rheaton Sorry for the trouble!

rheaton · 2016-02-12T21:36:08Z

NP! We were actually bemoaning that it was an older version of rubocop this morning. 😁 Love those Friday deploys, haha.

jpignata mentioned this pull request Jan 31, 2016

Update rubocop to 0.36.0 #46

Closed

Bump rubocop to 0.36.0

472aa51

See: https://github.com/bbatsov/rubocop/blob/master/CHANGELOG.md#0360-14012016

jpignata force-pushed the jp/upgrade-rubocop branch from e88e4af to 472aa51 Compare January 31, 2016 17:48

jpignata force-pushed the jp/upgrade-rubocop branch 5 times, most recently from 333b7d8 to e85e236 Compare February 1, 2016 03:33

jpignata force-pushed the jp/upgrade-rubocop branch from e85e236 to db179d4 Compare February 1, 2016 03:57

jpignata added 2 commits February 1, 2016 07:05

Convert suite to rspec

d6512a0

jpignata force-pushed the jp/upgrade-rubocop branch from db179d4 to 44871a0 Compare February 1, 2016 12:05

Add coverage for Issue

266eb9d

jpignata force-pushed the jp/upgrade-rubocop branch from 44871a0 to 266eb9d Compare February 1, 2016 12:07

jpignata added 2 commits February 10, 2016 22:14

Bump to rubocop 0.37.1

562cea6

jpignata force-pushed the jp/upgrade-rubocop branch 3 times, most recently from 406c281 to 0e05f0b Compare February 11, 2016 03:48

jpignata changed the title ~~Bump rubocop to 0.36.0~~ Bump rubocop to 0.37.1 Feb 11, 2016

jpignata added 2 commits February 11, 2016 11:45

Revert to rubocop 0.36.0 config

25e9a15

Bump to rubocop 0.37.2

88974fc

jpignata force-pushed the jp/upgrade-rubocop branch from 0e05f0b to 88974fc Compare February 11, 2016 16:45

jpignata changed the title ~~Bump rubocop to 0.37.1~~ Bump rubocop to 0.37.2 Feb 11, 2016

jpignata added a commit that referenced this pull request Feb 11, 2016

Merge pull request #48 from codeclimate/jp/upgrade-rubocop

fcfe222

Bump rubocop to 0.37.2

jpignata merged commit fcfe222 into master Feb 11, 2016

jpignata deleted the jp/upgrade-rubocop branch February 11, 2016 21:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump rubocop to 0.37.2 #48

Bump rubocop to 0.37.2 #48

jpignata commented Jan 31, 2016

jpignata commented Jan 31, 2016

wfleming commented Jan 31, 2016

jpignata commented Jan 31, 2016

jpignata commented Feb 1, 2016

jpignata commented Feb 1, 2016

jpignata commented Feb 1, 2016

wfleming commented Feb 1, 2016

jpignata commented Feb 1, 2016

ABaldwinHunter commented Feb 2, 2016

wfleming commented Feb 2, 2016

jpignata commented Feb 11, 2016

gdiggs commented Feb 11, 2016

wfleming commented Feb 11, 2016

jpignata commented Feb 11, 2016

wfleming commented Feb 11, 2016

ABaldwinHunter commented Feb 11, 2016

rheaton commented Feb 12, 2016

rheaton commented Feb 12, 2016

jpignata commented Feb 12, 2016

rheaton commented Feb 12, 2016

jpignata commented Feb 12, 2016

rheaton commented Feb 12, 2016

jpignata commented Feb 12, 2016

rheaton commented Feb 12, 2016

Bump rubocop to 0.37.2 #48

Bump rubocop to 0.37.2 #48

Conversation

jpignata commented Jan 31, 2016

jpignata commented Jan 31, 2016

wfleming commented Jan 31, 2016

jpignata commented Jan 31, 2016

jpignata commented Feb 1, 2016

jpignata commented Feb 1, 2016

jpignata commented Feb 1, 2016

wfleming commented Feb 1, 2016

jpignata commented Feb 1, 2016

ABaldwinHunter commented Feb 2, 2016

wfleming commented Feb 2, 2016

jpignata commented Feb 11, 2016

gdiggs commented Feb 11, 2016

wfleming commented Feb 11, 2016

jpignata commented Feb 11, 2016

wfleming commented Feb 11, 2016

ABaldwinHunter commented Feb 11, 2016

rheaton commented Feb 12, 2016

rheaton commented Feb 12, 2016

jpignata commented Feb 12, 2016

rheaton commented Feb 12, 2016

jpignata commented Feb 12, 2016

rheaton commented Feb 12, 2016

jpignata commented Feb 12, 2016

rheaton commented Feb 12, 2016