Skip to content

Rails 6.1 v2 - #8

Open
kobsy wants to merge 5 commits into
boblail:masterfrom
cph:rails-6.1-v2
Open

Rails 6.1 v2#8
kobsy wants to merge 5 commits into
boblail:masterfrom
cph:rails-6.1-v2

Conversation

@kobsy

@kobsy kobsy commented Dec 13, 2021

Copy link
Copy Markdown
Contributor

Summary

This builds upon the work that Luke did in #7 but addresses some under-the-hood changes to ActiveRecord that took place between Rails 6.0 and Rails 6.1. It also moves CI from Travis (not running anymore without a config update) to GHA (which is free for open-source projects like this). 🙂

Closes #7

@kobsy

kobsy commented Dec 13, 2021

Copy link
Copy Markdown
Contributor Author

It doesn't look like I can request a review, but would you mind giving it a look when you get the chance, @boblail? 🙂

@boblail
boblail self-requested a review December 23, 2021 16:37

@boblail boblail left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I've removed Travis from the required status checks in Branch Protection.

Should we consider this an alternative to #7? (It looks it doesn't just build on the other PR but squashes some commits too?)

Comment thread lib/hayfork/statement.rb Outdated
end

def types_method
before_rails61? ? :types : :klass

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect this to work — does it?

Suggested change
before_rails61? ? :types : :klass
ActiveRecord::VERSION < "6.1" ? :types : :klass

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That didn't work, but ActiveRecord.version < Gem::Version.new("6.1") did. 🙂

@kobsy

kobsy commented Jan 3, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for taking a look, @boblail. I updated the version check to be a little more elegant, per your suggestion.

This would be intended to supersede #7, as it makes a few additional changes to ensure the gem is Rails 6.1+ ready.

Comment thread lib/hayfork/binding.rb Outdated
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.

3 participants