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

DEVX-7780: Update Logger Type Signatures #290

Merged
merged 6 commits into from
Oct 10, 2023

Conversation

superchilled
Copy link
Contributor

@superchilled superchilled commented Oct 10, 2023

This PR updates some Sorbet type signatures for Logger creation and assignment. The motivation for this change is that in Rails 7.1.0 a change has been made to Rails.logger so that it no longer inherits from Ruby's std lib Logger class. This means that Rails.logger in 7.1.0 is invalid according to the type signatures in the Vonage Ruby SDK. See this issue for more detail.

The specific fix is to update the type signatures for logger creation and assignment to conditionally allow Rails 7.1's ActiveSupport::BroadcastLogger as a type if it is defined. The signatures updated are:

  • Vonage::Config#logger=
  • Vonage::Logger#initialize

Thanks to @ohbarye for the detailed bug report and proposed fix.

@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2023

Codecov Report

Merging #290 (6a70b8e) into main (b3122cb) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 6a70b8e differs from pull request most recent head d4c1a06. Consider uploading reports for the commit d4c1a06 to get more accurate results

@@           Coverage Diff           @@
##             main     #290   +/-   ##
=======================================
  Coverage   97.93%   97.94%           
=======================================
  Files         108      108           
  Lines        1988     1991    +3     
=======================================
+ Hits         1947     1950    +3     
  Misses         41       41           
Files Coverage Δ
lib/vonage/config.rb 97.93% <100.00%> (+0.04%) ⬆️
lib/vonage/logger.rb 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@superchilled superchilled marked this pull request as ready for review October 10, 2023 14:46
@superchilled superchilled merged commit 8086a92 into main Oct 10, 2023
17 checks passed
@superchilled superchilled deleted the devx-7780-update-logger-type-signature branch October 10, 2023 19:16
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