General cleanup, mostly focused on attributes/*

[pd]

Driven mostly by running flog, flay and reek locally; CodeClimate comparison is available here.

Showing 39 changed files with 528 additions and 605 deletions., but that's largely due to removing a level or two of indents from the FooAttribute classes.

Primary changes:

1. Almost all FooAttribute classes started with if data.is_a?(Foo); converted these to return unless data.is_a?(Foo). See bc04d2f.
2. Introduce locals for repeated traversals of objects (eg, current_schema.schema).
3. minLength, maxItems, maximum, etc etc all derive from a LimitAttribute class, and live in the same file (175e7dc). I don't really like growing the class hierarchy, and LimitAttribute itself is now too abstract for my taste, but each of those classes were previously copypasta with mostly one or two keywords changed.
4. I can't get 1.8 to run locally (segfaults loading rubygems!), but I think I've fixed the NoMethodError: undefined method 'keys' for [["c", "c"]]:Array error there.

If this is too big/insane to review reasonably, lemme know. I could pull the unnesting commit out of this and do it in a separate PR instead, which might make this more grokkable.

[iainbeeston]

Wow... Great work, it was on my list of things to do something like this (although I was going to limit it to just the properties and properties v4 attribute).

Unfortunately it is a bit hard to code review. I've had a (very) quick read through and I didn't see anything unnerving.

I would be a lot more confident about merging this if I had an idea of what code coverage we have for attributes. Have you tried running simplecov over this branch and checked how well covered the attributes are? (If not I would suggest using it as a guide to add a few more tests in any coverage "black-holes")

This is definitely the right direction to be going in though, IMO

[pd]

Coverage is pretty decent around attributes. I've pushed two separate branches to my fork for before/after coverage metrics (on the 2.1 run in this case, but it's roughly the same for 1.9, 2.0, jruby, rbx): master-coverage and climate-coverage.

There are uncovered paths, but nothing new that I can identify except some of my raise NotImplementedError method bodies, since those shouldn't ever be hit. Some of the gaps on master are now covered since they've been combined; eg, properties_v4 previously lacked coverage of strict + patternProperties, but it seems v3 did hit that, so v4 gets it for free now.

[pd]

One unnerving thing, though unrelated to this branch it seems: once we enable simplecov, an IPv4 format test is reliably failing. This issue already exists on master.

Both branches are listed here, and an example of the failure is here.

[iainbeeston]

Well ignoring, that I'd be happy to see this merged, pending more people taking a look=

[RST-J]

I waded through, except for f23f39b - I simply don't have the mental capability left to finish this thoroughly now ;-).
Really a great improvement, nice work @pd
While reviewing, I mostly compared to what the old code did and what I expected from the context in which the refactoring took place. For the greater changes, I hope we can also rely on the test suite.

[RST-J]

👍