shoulda-context icon indicating copy to clipboard operation
shoulda-context copied to clipboard

Add ruby 3

Open corincerami opened this issue 2 years ago • 7 comments

corincerami avatar Aug 28 '23 14:08 corincerami

Thank you for your contribution. It appears that we need to include support for Ruby 3 in snowglobe in order to resolve the CI issue. What are your thoughts, @mcmire? Would you prefer an alternative approach, or are you open to a PR addressing this?

vsppedro avatar Aug 28 '23 15:08 vsppedro

@VSPPedro yeah, I noticed the same with Snowglobe. Interestingly, if I just change one method on version 0.3.0 of Snowglobe

# lib/snowglobe/project_command_runner.rb

def run_rake_tasks(*tasks)
  options = tasks.last.is_a?(Hash) ? tasks.pop : {}
  args = ["bundle", "exec", "rake", *tasks, "--trace"] + [options]
  run(*args)
end

to

# lib/snowglobe/project_command_runner.rb

def run_rake_tasks(*tasks)
  options = tasks.last.is_a?(Hash) ? tasks.pop : {}
  args = ["bundle", "exec", "rake", *tasks, "--trace"]
  run(*args, **options)
end

then the shoulda-context test suite passes.

Oddly however, this code is all different on snowglobe/master, which I think is Ruby 3 compatible but then raises another error that it can't find a run_n_unit_test_suite method.

corincerami avatar Aug 28 '23 15:08 corincerami

@VSPPedro I realized I never responded here, sorry about that. I'm fine with bumping snowglobe to Ruby 3. I can give you access if need be.

mcmire avatar Nov 14 '23 23:11 mcmire

Oh I see, there are some breaking (and potentially buggy?) changes on the master version of Snowglobe. Hmm, okay. Well, I can still grant access, but I'll also try to look at that soon so I can unblock a fix here.

mcmire avatar Nov 15 '23 00:11 mcmire

@mcmire, no problem. I can help you with that.

vsppedro avatar Nov 16 '23 17:11 vsppedro

@VSPPedro Cool, just sent you an invite to the repo.

mcmire avatar Nov 16 '23 20:11 mcmire

Any update here? Shoulda tests are nearly 4 years behind ruby and I'm starting to find it difficult to run them locally at all.

EDIT: Upon a closer look, this PR was seemingly spawned for exactly the same reason that I opened #102. Feel free to close my PR if this can be merged.

martinemde avatar Jul 14 '24 18:07 martinemde