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

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jabawack81
Copy link

as mentioned in the issue
#237 by @espen, the ruby-statistics gem updated the namespace of their gem from statistics to ruby-statistics
estebanz01/ruby-statistics@baadc9f and
estebanz01/ruby-statistics@00312b9 this commit is addressing the change

as mentioned in the issue
zombocom#237 the
`ruby-statistics` gem updated their namespce of their gem from
`statistics` to `ruby-statiscs`
estebanz01/ruby-statistics@baadc9f
and
estebanz01/ruby-statistics@00312b9
this commit is addressing the change
@espen
Copy link

espen commented Aug 15, 2024

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
Copy link

pcai commented Aug 18, 2024

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

@jabawack81
Copy link
Author

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
Copy link
Author

@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
Copy link

pcai commented Sep 17, 2024

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
Copy link

pcai commented Sep 17, 2024

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

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

Successfully merging this pull request may close these issues.

3 participants