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 gem and CI for more recent ruby versions #8

Closed
wants to merge 12 commits into from

Conversation

larskanis
Copy link
Member

I noticed this while trying to fix the rvm CI.
Would be nice to have a new release on rubygems.org so that it can be used in the rvm CI.

kind of:

EDEPRECATED: global use of wont_equal from test/gem-empty/specification_and_version_test.rb:24. Use _(obj).wont_equal instead. This will fail in Minitest 6.
Kind of:
  warning: `if' at the end of line without an expression
Kind of:
  test/gem-empty/command_test.rb:8: warning: method redefined; discarding old remove_spec
it is no longer working
@larskanis
Copy link
Member Author

@larskanis
Copy link
Member Author

@pkuczynski Could you please review this PR?

@wintersolutions
Copy link

Running into issues with File.exists? being removed in Ruby v3.2.0 would be really appreciated if this gem was updated. Thanks for the PR.

subject.defaults_str.class.must_equal(String)
subject.description.class.must_equal(String)
subject.program_name.class.must_equal(String)
_(subject.arguments.class).must_equal(String)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's these _ calls for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As written in the commit message, this was the minitest recommendation:

DEPRECATED: global use of wont_equal from test/gem-empty/specification_and_version_test.rb:24. Use _(obj).wont_equal instead. This will fail in Minitest 6.

- os: ubuntu
ruby: "3.2"
- os: ubuntu
ruby: "2.5"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.5 is old now ...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the tests runs sufficiently fast, we could include a lot more versions here...

if
RUBY_VERSION == "2.0.0" # check Gemfile
then
if RUBY_VERSION == "2.0.0" # check Gemfile

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test no longer run on 2.0.0 as far as I can see, which means no code coverage....

@mjobin-mdsol
Copy link

Thanks for porting to Github actions, this is sure a good change.

last run:
https://github.com/larskanis/gem-empty/actions/runs/3952435065

Copy link

@mathieujobin mathieujobin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see inline comments

Since the CI runs are reasonable fast.
since this is the oldest version we test in CI
and remove ruby-2.0 specifics
since it fails on ruby-2.7 and seems to be no longer necessary for the tests to succeed.
@larskanis
Copy link
Member Author

I addressed the comments above. CI run looks like so now: https://github.com/larskanis/gem-empty/actions/runs/4207194540/jobs/7301680135

Coverage reporting to coveralls.io doesn't work and prints some warning into the log. Can coveralls be removed?

@mojavelinux
Copy link

rvm's gemset empty command is broken on Ruby 3.2 without this change. If this PR is not ready to be merged, would it be reasonable to accept a simpler PR that just fixes the File.exists? call so a release can be made to get this command working again?

@larskanis
Copy link
Member Author

I separated all the changes into district commits, so that cherry picking is also an option. Although I think that all the changes are reasonable to bring the gem to an up-to-date level.

@pkuczynski
Copy link
Member

Let me have a look at proposed changes...

pkuczynski added a commit that referenced this pull request May 26, 2023
This was referenced May 26, 2023
@pkuczynski
Copy link
Member

@mojavelinux 1.2.0 released. Let me know if you find any issue

@pkuczynski
Copy link
Member

@larskanis I am not sure what did I missed when cherrypicking from this PR, but 3.3 and head are failing. Any idea why?

https://github.com/rvm/gem-empty/actions/runs/5095320724

@pkuczynski
Copy link
Member

Thanks for your work on this @larskanis. Hope you didnt mind me cherypicking form this PR instead of merging. I wanted to keep more backward compatibility...

@pkuczynski pkuczynski closed this May 26, 2023
@mojavelinux
Copy link

Success! Thanks a bunch!

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.

7 participants