MiniProfiler / rack-mini-profiler

Profiler for your development and production Ruby rack apps.
MIT License
3.7k stars 402 forks source link

Make MiniProfiler.profile_method compatible with Ruby 3 #481

Closed OsamaSayegh closed 3 years ago

OsamaSayegh commented 3 years ago

Fix for https://github.com/MiniProfiler/rack-mini-profiler/issues/480.

We can't pass **kwargs unconditionally because it would break backward compatibility with ruby < 2.7:

~ » cat ~/test.rb
class A
  def t(a, b = 2)
    { a: a, b: b }
  end
end

A.send :alias_method, 'old_t', 't'
A.send :define_method, 't' do |*args, **kwargs, &blk|
  self.send 'old_t', *args, **kwargs, &blk
end

puts RUBY_VERSION
puts A.new.t(10)

~ » RBENV_VERSION=2.7.0 rbenv exec ruby ~/test.rb
2.7.0
{:a=>10, :b=>2}  # <=== 👍 

~ » rbenv exec ruby ~/test.rb
2.6.6
{:a=>10, :b=>{}} # <=== 👎 
codecov-io commented 3 years ago

Codecov Report

Merging #481 (1b229ae) into master (8711a00) will decrease coverage by 1.18%. The diff coverage is 35.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #481      +/-   ##
==========================================
- Coverage   87.69%   86.50%   -1.19%     
==========================================
  Files          18       18              
  Lines        1243     1245       +2     
==========================================
- Hits         1090     1077      -13     
- Misses        153      168      +15     
Impacted Files Coverage Δ
lib/mini_profiler/profiling_methods.rb 56.43% <35.13%> (-9.45%) :arrow_down:
lib/mini_profiler/timer_struct/page.rb 88.13% <0.00%> (-0.39%) :arrow_down:
lib/mini_profiler/timer_struct/sql.rb 85.00% <0.00%> (-0.37%) :arrow_down:
lib/mini_profiler/profiler.rb 84.94% <0.00%> (-0.21%) :arrow_down:
lib/mini_profiler/snapshots_transporter.rb 91.66% <0.00%> (-0.12%) :arrow_down:
lib/mini_profiler/client_settings.rb 94.11% <0.00%> (-0.09%) :arrow_down:
lib/mini_profiler/gc_profiler.rb 96.20% <0.00%> (-0.05%) :arrow_down:
lib/mini_profiler/timer_struct/request.rb 98.98% <0.00%> (-0.02%) :arrow_down:
lib/mini_profiler/timer_struct/custom.rb 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8711a00...1b229ae. Read the comment docs.

SamSaffron commented 3 years ago

I think cause this is so perf sensitive and we are simply forwarding calls we should use ruby2_keywords

https://bugs.ruby-lang.org/issues/16897

then longer term once we only target Ruby 3 we can start using ... delegation.

OsamaSayegh commented 3 years ago

Ok I've updated this to use ruby2_keywords.

SamSaffron commented 3 years ago

PERFECT, nice simple patch :heart:

Will push out a new version now.