Skip to content
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

Add compatibility with rails 5 #54

Merged
merged 1 commit into from Mar 5, 2016
Merged

Add compatibility with rails 5 #54

merged 1 commit into from Mar 5, 2016

Conversation

@mgrachev
Copy link
Contributor

@mgrachev mgrachev commented Jan 21, 2016

I get the following error when using this gem with rails 5.0.0.beta1:

/Users/mgrachev/mm/vendor/bundle/gems/marginalia-1.3.0/lib/marginalia.rb:55:in `exec_query_with_marginalia': wrong number of arguments (4 for 1..3) (ArgumentError)
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/connection_adapters/abstract/database_statements.rb:375:in `select_prepared'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/connection_adapters/abstract/database_statements.rb:39:in `select_all'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/connection_adapters/abstract/query_cache.rb:70:in `select_all'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/querying.rb:39:in `find_by_sql'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/relation.rb:691:in `exec_queries'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/relation.rb:572:in `load'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/relation.rb:252:in `to_a'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/relation/delegation.rb:39:in `map'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/migration.rb:1002:in `block in get_all_versions'
from /Users/mgrachev/mm/vendor/bundle/gems/activesupport-5.0.0.beta1/lib/active_support/deprecation/reporting.rb:34:in `silence'
from /Users/mgrachev/mm/vendor/bundle/gems/activesupport-5.0.0.beta1/lib/active_support/deprecation/instance_delegator.rb:19:in `silence'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/migration.rb:1000:in `get_all_versions'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/migration.rb:1014:in `needs_migration?'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/migration.rb:548:in `load_schema_if_pending!'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/migration.rb:563:in `block in maintain_test_schema!'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/migration.rb:794:in `suppress_messages'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/migration.rb:568:in `method_missing'
from /Users/mgrachev/mm/vendor/bundle/gems/activerecord-5.0.0.beta1/lib/active_record/migration.rb:563:in `maintain_test_schema!'

This PR adds compatibility with new version rails and fix this error.

@arthurnn
Copy link
Collaborator

@arthurnn arthurnn commented Jan 25, 2016

This will raise an syntax error on 1.9.3 https://travis-ci.org/basecamp/marginalia/jobs/103911750
Maybe, we should use an hash instead?
Thanks for the PR

@mgrachev
Copy link
Contributor Author

@mgrachev mgrachev commented Jan 25, 2016

Fixed

@arthurnn arthurnn self-assigned this Jan 25, 2016
@mgrachev
Copy link
Contributor Author

@mgrachev mgrachev commented Mar 5, 2016

@arthurnn All right with PR or need something to modify?

arthurnn pushed a commit that referenced this pull request Mar 5, 2016
Arthur Nogueira Neves
Add compatibility with rails 5
@arthurnn arthurnn merged commit 6f87b39 into basecamp:master Mar 5, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@arthurnn
Copy link
Collaborator

@arthurnn arthurnn commented Mar 5, 2016

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants