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
Test against Rails 5.2 #151
Conversation
gem 'rails', '~> 5.2.0' | ||
|
||
# Rails imposed mysql2 version contraints | ||
# https://github.com/rails/rails/blob/5-2-stable/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb#L4 |
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.
Line 6 seems to the appropriate line. (#6
) Could you please change the 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.
@orien sorry I do not quite get what you mean
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.
If you follow this link it highlights the wrong line:
https://github.com/rails/rails/blob/5-2-stable/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb#L4
.travis.yml
Outdated
env: DB=sqlite | ||
- rvm: 2.5 | ||
gemfile: spec/support/gemfiles/Gemfile.rails-5.2.x | ||
env: DB=postgres |
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.
👏
To reduce build times, could we remove these two Rails 5.1 entries from the matrix:
- rvm: 2.5
gemfile: spec/support/gemfiles/Gemfile.rails-5.1.x
env: DB=sqlite
- rvm: 2.5
gemfile: spec/support/gemfiles/Gemfile.rails-5.1.x
env: DB=postgres
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.
@orien I would love to leave it there to cover these 2 adapters.
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.
They'll still be covered by the Rails 5.2 entries you've just added.
We should add Rails 5.2 to the change log too. https://github.com/envato/double_entry/blob/master/CHANGELOG.md |
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.
Awesome!
No description provided.