Skip to content

Added support for Rails 8#765

Closed
basex wants to merge 0 commit intonesquena:masterfrom
basex:master
Closed

Added support for Rails 8#765
basex wants to merge 0 commit intonesquena:masterfrom
basex:master

Conversation

@basex
Copy link
Copy Markdown

@basex basex commented Nov 12, 2024

Followed the same as #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
Copy link
Copy Markdown
Collaborator

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
Copy link
Copy Markdown
Collaborator

@basex please rebase, specs should pass

@basex
Copy link
Copy Markdown
Author

basex commented Nov 13, 2024

done @tagliala Thanks

@tagliala
Copy link
Copy Markdown
Collaborator

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

@tagliala
Copy link
Copy Markdown
Collaborator

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
Copy link
Copy Markdown
Collaborator

tagliala commented Nov 14, 2024

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@1c45233 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
Copy link
Copy Markdown
Author

basex commented Nov 14, 2024

Discarding the commits in my fork automatically closed this MR. New PR from another branch here #768

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants