-
Notifications
You must be signed in to change notification settings - Fork 151
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
em-synchrony 1.0.3 and 1.0.4 do not work with ActiveRecord 5.0 #201
Comments
Hello. If you're (or someone) still ineresting in, I've created a branch with ActiveRecord 5 fix: gem 'em-synchrony', github: 'Xanders/em-synchrony', branch: 'active_record_5', require: %w[em-synchrony em-synchrony/mysql2 em-synchrony/activerecord em-synchrony/em-http em-synchrony/fiber_iterator] # just a sample, fill the require list by yourself Also it looks like tests was fixed two months ago: 1a232d2 |
@Xanders Hey, is there a reason you've branched your fix not from master, but from a proposed pull request branch? |
Any particular reason for that? #190 would have to be rebased first. |
@dgutov Is there any other way to create PR with code using code from other PR except of copy-paste all of that code? I don't understand what the problem now, sorry. |
I'm asking why you've used the approach from that PR and not followed the approach that we currently have in master. |
@dgutov I've tried current master code (both active_record.rb and active_record_4.rb) with AR5 and had a lot of strange errors, didn't know how to fix them. But when i tried fork from #190 it was almost worked except of some moments, that i fixed. I assumed fork is more recent version of synchrony/ar then current master, so I'm forked it, not master. |
It's not more recent, no. But it's possible that it uses a better approach, considering you've had to fix fewer places. |
@dgutov So I was wrong, sorry. But I think code from that fork is much simpler and more stable, so I still prefer it. |
Do the existing tests pass? |
@dgutov Only my app's tests. I didn't run synchrony's tests, I don't have much time. It was just a quick fix to force my app working. |
Please let us know the result when you get around to it. |
Hey folks, could you please try the current em-synchrony master? See if it works okay with your AR5 based applications. |
For both em-synchrony 1.0.3 and 1.0.4 I get the same
undefined local variable or method 'current_connection_id'
error withactiverecord 5.0.0.beta1
.I pulled down em-synchrony to add a patch, but after bundling and trying to run the test suite ruby 2.2.3 segfaulted.
The text was updated successfully, but these errors were encountered: