Skip to content

Add 'logger' as dependency in gemspec#401

Open
leoarnold wants to merge 1 commit into
roidrage:masterfrom
leoarnold:leoarnold/gemspec
Open

Add 'logger' as dependency in gemspec#401
leoarnold wants to merge 1 commit into
roidrage:masterfrom
leoarnold:leoarnold/gemspec

Conversation

@leoarnold

Copy link
Copy Markdown

Follow up to #399.

@johnnagro

Copy link
Copy Markdown
Collaborator

Thanks for the follow-up @leoarnold - consolidating logger into the gemspec (and dropping the Gemfile duplicate) makes sense.

One thing though: unlike base64/bigdecimal/mutex_m (which we only need so older Rails can load in our test matrix), logger is required by lograge's own runtime code (require 'logger' in lib/lograge.rb). Since we support activesupport >= 4, and activesupport didn't declare logger as a runtime dependency until Rails 7.1, an app on Rails ≤ 7.0 with Ruby 3.5+ (where logger is no longer a default gem) could fail to load lograge but, a development_dependency wouldn't help those users.

What do you think about making it s.add_dependency 'logger' (runtime) instead? That protects consumers, and it can stay out of the dev block / Gemfile entirely. Happy to merge once that's adjusted.

@johnnagro

Copy link
Copy Markdown
Collaborator

@leoarnold lemme figure out how we might relax/ignore that rubocop rule breaking your build.

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