upgrade roar to current version

[ashtonthomas]

We define a subclass of Roar::Decorator and include some modules which have had their namespace changed in roar 1.0.0.

Roar has seen a lot of new developments recently with support for hypermedia attributes. These changes along with the general value of not restricting dependency versions when we don't need to should merit this pull request.

In this pull request:

• unpin roar (remove the pessimistic version control)
• leave the minimum version at 0.12.0 as we need this for our decorator feature
• support both pre/post 1.0.0 (currently using a LoadError rescue)

[ashtonthomas]

We really only need to bump roar to 1.0.0 to get the change to namespaces

[darbyfrey]

Could we change it to support both pre 1.0 and 1.x versions of Roar? Ideally, I think the individual services should decide which version they want to use and Napa should support all versions

[ashtonthomas]

@darbyfrey, is there a pattern for supporting different versions like you suggest?

I used a LoadError rescue and this seems to work. I've updated this pull request to support pre and post namespace refactoring. I have change the gemspec to allow higher version changes. Also, the Coercion didn't need a namespace prefix to work.

I tested this with an internal app and set roar to ~> 0.12.0 (0.12.9) as well as 1.0.0 and HEAD versions of roar.

Thoughts?

[darbyfrey]

While this would work, it's not very clear why you're doing the begin/rescue, etc.

A simple way to do it would be wrap a conditional around the VERSION value. So, something like:

if Roar::VERSION > 'x.y.z'
  # load files
else
  # load other files
end

Are the dependencies clear enough that you could use this approach?

[ochagata]

This could use a specific pessimistic approach.

So we could do gem.add_dependency 'roar', '>= 0.12.0', '< 2.0.0'

[ochagata]

I agree that it shouldn't cause a LoadError and instead use an if/else clause.

[ashtonthomas]

@darbyfrey I've used your approach. Yeah, that makes perfect sense. I was just totally unsure of the better way to do it. Thanks for the tip.

@ochagata, I've added the pessimistic upper bound limiting roar to 1.x

[ochagata]

👍

[darbyfrey]

This will set a global constant in any app that uses Napa.

Any reason you can't pull this conditional around the include within the Representer class?

[ashtonthomas]

that's a good call. will look into doing this

[ashtonthomas]

@darbyfrey, I removed the constant, but I'm still not sure if this is the most appropriate method

[darbyfrey]

This seems fine to me. You could add a helper method to wrap the version check to be more explicit about what you're doing, but that's not totally necessary.

+1 from me. Anyone else?

[shaqq]

LGTM +1

[darbyfrey]

@ashtonthomas, will you add a note for this change into the CHANGELOG? Then we can merge it in.

[ashtonthomas]

@darbyfrey, sorry for the delay. I rebased with master and added the changelog

[darbyfrey]

Thanks @ashtonthomas!