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

Add logger as a dependency, drop ostruct for tests #1010

Merged

Conversation

olleolleolle
Copy link
Contributor

@olleolleolle olleolleolle commented Aug 8, 2024

This PR avoids a Ruby warning.

There was a warning in ruby-head (3.4) that in Ruby 3.5 logger and ostruct will not be built-in gems, but regular gems.

/home/runner/work/dalli/dalli/test/helper.rb:13: warning: logger was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0. Add logger to your Gemfile or gemspec.
/home/runner/work/dalli/dalli/test/helper.rb:14: warning: ostruct was loaded from the standard library, but will no longer be part of the default gems since Ruby 3.5.0. Add ostruct to your Gemfile or gemspec.

This change avoids that warning.


In addition, the ostruct dependency wasn't in actual use, just require 'ostruct' statements. It was possible to fully omit that and pass all tests.

A second commit with a linting change was added, using the autocorrect feature of RuboCop.

@olleolleolle olleolleolle changed the title Add logger as a dependency Add logger as a dependency, ostruct to the Gemfile for tests Aug 8, 2024
@petergoldstein
Copy link
Owner

@olleolleolle can you please address the Rubocop warnings? Thanks.

There was a warning in ruby-head (3.4) that in Ruby 3.5 logger and
ostruct will not be built-in gems, but regular gems.

This change avoids that warning.

The ostruct dependency was be omitted.
@olleolleolle olleolleolle force-pushed the add-logger-ostruct-to-gemfile branch from 9eea4f0 to 6032ca5 Compare August 13, 2024 06:13
@olleolleolle
Copy link
Contributor Author

olleolleolle commented Aug 13, 2024

@petergoldstein Thanks, I revisited the change, and was able to remove the ostruct, and pass linting.

EDIT: I guess the port-finding is a little finicky, and sometimes trips tests up. Thought of this and remembered #1000 (draft) which tried to address this.

@olleolleolle olleolleolle changed the title Add logger as a dependency, ostruct to the Gemfile for tests Add logger as a dependency, drop ostruct for tests Aug 13, 2024
@petergoldstein petergoldstein merged commit 718cacb into petergoldstein:main Sep 16, 2024
21 checks passed
@olleolleolle olleolleolle deleted the add-logger-ostruct-to-gemfile branch September 16, 2024 21:04
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.

2 participants