search

nesquena / rabl

General ruby templating with json, bson, xml, plist and msgpack support
http://blog.codepath.com/2011/06/27/building-a-platform-api-on-rails/
MIT License
3.64k stars 334 forks source link

Setup Appraisal for all permutation of Ruby and ActiveSupport #667

Closed krzysiek1507 closed 7 years ago

krzysiek1507 commented 8 years ago

Should we drop support for Ruby < 2.0 (or 2.1) and Rails < 4.0 (or 4.1)?

krzysiek1507 commented 8 years ago

@nesquena what about this PR? Fails aren't related to this PR.

databyte commented 8 years ago

It doesn't seem to actually be testing anything though. It just loads up Gemfiles...

We have fixtures for rails here: https://github.com/nesquena/rabl/tree/master/fixtures

It'd be better if we tested out each version of rails through the fixtures. It doesn't look like we've been keeping up with it though...

krzysiek1507 commented 8 years ago

@databyte do you know that 4 of 5 builds fail? These changes allow us to see if tests pass for multiple configurations. We can also add Sinatra or whatever.

databyte commented 8 years ago

668 is addressing that by fixing the RABL code itself vs just the testing framework and without adding another dependency. If it meets your needs, we may not need your PR.

If there's added value to your PR though by way of providing functionality above and beyond the current fixtures, that'd be great. But by reading through it, I think both this PR and #668 provide a fix to the build issues and in #668's case, a cleaner setup for when RABL hooks into the different versions of Rails.

domcleal commented 8 years ago

The changes in #668 are pretty simplistic, they only run the core unit tests against the latest version of Rails for each Ruby version (so only versions 4.2 and 5.0). Running it for each permutation like this does is also useful to ensure the older Rails versions get tested.

databyte commented 8 years ago

Sounds like both PR's are useful then because the Rails fixtures are running real integration tests through a Rails controller and template rendering whereas the base test suite doesn't. It's just rendering templates through its engine. And this PR is just for the base tests, it's not interacting with the fixtures.

Looks like this setup fails 1.9.3 with AS 2.3, I guess it's up to @nesquena but I don't see any reason to support Rails 2.x. If it doesn't fail for Rails 3.x, I don't see any reason why not to continue supporting n-1 back too. If someone wants to continue using it with 2.x, it hasn't been working already anyway and they can lock it.

If you drop the 2.x support in the build and it builds cleanly, I'll merge it for you.

krzysiek1507 commented 8 years ago

I've added activesupport 2.x because of this

databyte commented 8 years ago

Curious then, we should probably fix 2.3 before merging. Does anyone know if #656 addresses some (or all) of the breaks?

krzysiek1507 commented 8 years ago

I think this failed case isn't related to this PR. What's the plan for supporting Ruby < 2 and Rails < 4?

databyte commented 8 years ago

Yeah, I agree that it's not related. Later today when I have an hour to dedicate to this, I'll merge your PR, fix the 2.3 issue, probably pull in #656 if its related and just make everything stable again.

Long term support is up to @nesquena but if it works with a couple syntax tweaks, I'm not personally going out of my way to drop support unless some new feature just makes backwards compatibility too hard or there's some major speed improvements for dropping support.

I may lobby to drop support if fixing the tests for 2.3 end up with a (╯°□°）╯︵ ┻━┻

But I can't say that until I've given it a shot.

krzysiek1507 commented 8 years ago

@databyte what about this?