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

Fix Sequel 5 issues #121

Merged
merged 5 commits into from Dec 1, 2017

Conversation

Projects
None yet
1 participant
@shioyama
Owner

shioyama commented Dec 1, 2017

Fixes #94

@shioyama

This comment has been minimized.

Show comment
Hide comment
@shioyama

shioyama Dec 1, 2017

Owner

So the good news is, Sequel 5 is mostly working fine. The only thing that is failing is query support for the Table backend, because there is code which sets an instance variable on a dataset, and this is no longer possible since datasets in Sequel are now frozen by default and cannot be unfrozen by simply duping them, because dup simply returns (the frozen) dataset.

The problem in Mobility is here, where we set an instance variable to avoid joining the same table more than once. This is admittedly not very nice code, but it serves an important purpose...

Owner

shioyama commented Dec 1, 2017

So the good news is, Sequel 5 is mostly working fine. The only thing that is failing is query support for the Table backend, because there is code which sets an instance variable on a dataset, and this is no longer possible since datasets in Sequel are now frozen by default and cannot be unfrozen by simply duping them, because dup simply returns (the frozen) dataset.

The problem in Mobility is here, where we set an instance variable to avoid joining the same table more than once. This is admittedly not very nice code, but it serves an important purpose...

shioyama added some commits Dec 1, 2017

@shioyama shioyama changed the title from [WIP] Fix Sequel 5 issues to Fix Sequel 5 issues Dec 1, 2017

@shioyama shioyama merged commit aab4697 into master Dec 1, 2017

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
codeclimate All good!
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@shioyama

This comment has been minimized.

Show comment
Hide comment
@shioyama

shioyama Dec 1, 2017

Owner

Travis was being stupid about a couple builds, which seemed to be stuck, so I just merged this. It's passing anyway. Full support for Sequel 5.0 - yay!

Owner

shioyama commented Dec 1, 2017

Travis was being stupid about a couple builds, which seemed to be stuck, so I just merged this. It's passing anyway. Full support for Sequel 5.0 - yay!

@shioyama shioyama deleted the fix_sequel_5_issues branch Dec 1, 2017

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