-
Notifications
You must be signed in to change notification settings - Fork 4
Rails 6.1 v2 #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Rails 6.1 v2 #8
Conversation
|
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
left a comment
There was a problem hiding this comment.
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?)
lib/hayfork/statement.rb
Outdated
| end | ||
|
|
||
| def types_method | ||
| before_rails61? ? :types : :klass |
There was a problem hiding this comment.
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?
| before_rails61? ? :types : :klass | |
| ActiveRecord::VERSION < "6.1" ? :types : :klass |
There was a problem hiding this comment.
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. 🙂
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