search

zombocom / derailed_benchmarks

Go faster, off the Rails - Benchmarks for your whole Rails app
2.98k stars 144 forks source link

update requirement in `lib/derailed_benchamrks/stats_from_dir.rb` #238

Closed jabawack81 closed 1 month ago

jabawack81 commented 3 months ago

as mentioned in the issue https://github.com/zombocom/derailed_benchmarks/issues/237 by @espen, the ruby-statistics gem updated the namespace of their gem from statistics to ruby-statistics https://github.com/estebanz01/ruby-statistics/commit/baadc9fd3bd90c9ed14e2bba48b8eb806bfe1ff2 and https://github.com/estebanz01/ruby-statistics/commit/00312b9a329c742c2b851a182d3ab35bb86fe535 this commit is addressing the change

espen commented 3 months ago

Wont this break for those running older version of ruby-statistics? I would assume there would have to be a version checker when requiring or update the Gemfile to require v4 of ruby-statistics?

pcai commented 3 months ago

@espen makes a good point - this PR needs to also bump ruby-statistics requirement to >= 4.0

jabawack81 commented 3 months ago

yes, sorry when I opened this PR I was in firefighter mode and wanted to fix my deployment CI, when I have a free moment today I'll fix that

jabawack81 commented 2 months ago

@espen and @pcai done, my only concern is that I tried to run the test to check that this update didn't screw up anything but they crash even on the original main with:

/home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/activesupport-7.0.0/lib/active_support/message_verifier.rb:4: warning: base64 was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.4.0. Add base64 to your Gemfile or gemspec. Also contact author of activesupport-7.0.0 to add base64 into its gemspec.
/home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/railties-7.0.0/lib/rails/railtie.rb:246:in `initialize': Rails::Engine is abstract, you cannot instantiate it directly. (RuntimeError)
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/railties-7.0.0/lib/rails/railtie.rb:184:in `new'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/railties-7.0.0/lib/rails/railtie.rb:184:in `instance'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/railties-7.0.0/lib/rails/railtie.rb:223:in `method_missing'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/activesupport-7.0.0/lib/active_support/descendants_tracker.rb:90:in `descendants'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/activesupport-7.0.0/lib/active_support/callbacks.rb:923:in `block in define_callbacks'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/activesupport-7.0.0/lib/active_support/callbacks.rb:920:in `each'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/activesupport-7.0.0/lib/active_support/callbacks.rb:920:in `define_callbacks'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/railties-7.0.0/lib/rails/engine.rb:427:in `<class:Engine>'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/railties-7.0.0/lib/rails/engine.rb:349:in `<module:Rails>'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/railties-7.0.0/lib/rails/engine.rb:11:in `<top (required)>'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `require'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `block (2 levels) in replace_require'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/railties-7.0.0/lib/rails/application.rb:11:in `<top (required)>'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `require'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `block (2 levels) in replace_require'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/railties-7.0.0/lib/rails.rb:13:in `<top (required)>'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `require'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `block (2 levels) in replace_require'
    from /home/XXXX/dev/derailed_benchmarks_orig/test/test_helper.rb:9:in `<top (required)>'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `require'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `block (2 levels) in replace_require'
    from /home/XXXX/dev/derailed_benchmarks_orig/test/derailed_benchmarks/core_ext/kernel_require_test.rb:3:in `<top (required)>'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `require'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/3.3.0/bundled_gems.rb:74:in `block (2 levels) in replace_require'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/rake-13.2.1/lib/rake/rake_test_loader.rb:21:in `block in <main>'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/rake-13.2.1/lib/rake/rake_test_loader.rb:6:in `select'
    from /home/XXXX/.rbenv/versions/3.3.1/lib/ruby/gems/3.3.0/gems/rake-13.2.1/lib/rake/rake_test_loader.rb:6:in `<main>'
rake aborted!
Command failed with status (1)

Tasks: TOP => test
(See full trace by running task with --trace)

So I don't think this is related to my PR, but it is still concerning

pcai commented 2 months ago

Hmm the rails version constraint looks a bit unorthodox. I would expect it to read < 7 if the intent was 6.x or lower or < 8 if the intent was 7.x or lower. If im reading correctly, it allows the initial 7.0.0 but none of the patches, which might be the problem?

pcai commented 2 months ago

@deepakmahakale I see you helped maintain this repo recently, any thoughts on above?

pcai commented 1 month ago

I investigated the test failures and they are mostly* unrelated. I'm going to repair the bitrot in a separate PR to get main green again. Thanks for your contribution.

*We may need to do some branching to maintain support for legacy ruby versions