fix: instrumentation of Rails 7 #993
Conversation
jimeh
requested review from
ahayworth,
arielvalentin,
dazuma,
ericmustin,
fbogsany,
mwear and
robertlaurin
as code owners
|
|
| @@ -17,7 +17,7 @@ module ActionView | |||
| def self.subscribe(pattern = nil, callable = nil) | |||
| ActiveSupport::Notifications.subscribe(pattern, callable) | |||
| ::ActiveSupport::Notifications.notifier.synchronize do | |||
| if ::Rails::VERSION::MAJOR == 6 | |||
| if ::Rails::VERSION::MAJOR >= 6 | |||
There was a problem hiding this comment.
With rails/rails#43282 (not sure if it's included with the alpha) we don't even need to adjust the ordering of subscribers.
| if ::Rails::VERSION::MAJOR >= 6 | |
| if ::Rails::VERSION::MAJOR == 6 |
There was a problem hiding this comment.
Oh, nice. However that PR was merged just over 3 weeks after the latest alpha was published, so it's definitely not included right now.
There was a problem hiding this comment.
This looks fine to me - once rails/rails#43282 is in a released version, we can definitely consider dropping our subscription-ordering hacks for Rails 7 and beyond. 😄
|
@jimeh I approved the Actions workflow runs for you - thank you for being a first time contributor! - but now the tests seem to be failing. |
|
@ahayworth Thanks, and the test failures are rather annoying. I had it all passing locally. I'll try and resolve the failures a bit later today when I'm free :) |
ActionView instrumentation no longer fails by having it use the same subscriber logic as used for Rails 6. Also add Rails 7 Appraisals to all relevant gems which has any Rails Appraisals.
jimeh
force-pushed
the
add-rails-7-support
branch
from
c61950c to
7b2fc3d
Compare
|
Test failures were due to Rails 7.x requiring Ruby 2.7.0 or later. I've just pushed a fix (hopefully), and also rebased the PR on top of latest state of the |
ActionView instrumentation no longer fails by having it use the same
subscriber logic as used for Rails 6.
Also add Rails 7 Appraisals to all relevant gems which has any Rails
Appraisals.