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

Added support for Rails 8 #765

Closed basex closed 1 week ago

basex commented 1 week ago

Followed the same as https://github.com/nesquena/rabl/pull/752

I tested locally and works without issues.

In the future if we stop supporting Rails 2 we can check for Rails::VERSION::MAJOR. Example: Rails::VERSION::MAJOR > 6

tagliala commented 1 week ago

Hi, thanks for this PR.

Failures do not depend by this PR. I have some small changes to do to the CI, then I'll ask to rebase

tagliala commented 1 week ago

@basex please rebase, specs should pass

basex commented 1 week ago

done @tagliala Thanks

tagliala commented 1 week ago

All green, let me try if I remember how to run integration tests. Did you run it locally?

tagliala commented 1 week ago

Sorry, rabl needs some other changes that are not related to this PR.

I will make the changes elsewhere and then ask to rebase or cherry-pick your commit to preserve your authoring

tagliala commented 1 week ago

Thanks,

I've performed an integration test locally against rails 8 and it worked, however we now have two options

  1. I'll cherry-pick this commit in a new PR and make the change. You will be preserved as an author of the commit, but I'm not sure that your name will appear in the automatic release notes
  2. You create a new branch on your fork, possibly with a good name, let's say rails8, then:
    • Cherry pick basex/rabl@1c45233fdbb2149631d396104f57fae5d6464d81 on that branch
    • Reset your fork master branch on this gem's master
    • Rebase rails8 on master, submit a new PR

Point 2 would also apply as best practice for future PRs. It is required to have a single clean commit to merge here (I do not have the "rebase" feature available, maybe because it's the master branch on the remote fork)

Please also add the following at the top of CHANGELOG.md

## unreleased

* Add Rails 8 compatibility (@basex)
basex commented 1 week ago

Discarding the commits in my fork automatically closed this MR. New PR from another branch here https://github.com/nesquena/rabl/pull/768