fix: remove undef :broadcast since ActiveSupport::Logger dropped it #194
Conversation
|
Fixes #191 |
| @@ -4,7 +4,7 @@ module ActiveSupport | |||
| # More hacks to try and stop Rails from being it's own worst enemy. | |||
| class Logger | |||
| class << self | |||
| undef :logger_outputs_to?, :broadcast | |||
| undef :logger_outputs_to? | |||
There was a problem hiding this comment.
...and CI would probably fail, because we're undefining methods only to override it later, since it's not defined anymore and replaced with other API -- some functionality would be broken.
There was a problem hiding this comment.
quick & dirty commit just to run CI https://github.com/ojab/rails_semantic_logger/tree/rails-7.1, it's failing in unexpected place, but indeed failing https://github.com/ojab/rails_semantic_logger/actions/runs/6461173004/job/17540325867
There was a problem hiding this comment.
Looks similar to sidekiq/sidekiq@20cdf68, but didn't checked the code
There was a problem hiding this comment.
Made a PR to fix it rails/rails#49563, will wait a couple of days for feedback and [if accepted] would try to make a PR to rails_semantic_logger, probably with out-of-the-box monkeypatch.
There was a problem hiding this comment.
added 7.1 to the matrix and implemented @camsteffen's solution above
There was a problem hiding this comment.
Made a PR to fix it rails/rails#49563, will wait a couple of days for feedback and [if accepted] would try to make a PR to
rails_semantic_logger, probably with out-of-the-box monkeypatch.
looks like a patch was merged into Rails
rails/rails#49621
There was a problem hiding this comment.
running
gem "rails_semantic_logger", git: "https://github.com/Amnesthesia/rails_semantic_logger.git", branch: "fix/rails7.1"
gem "rails", git: "https://github.com/rails/rails.git", branch: "7-1-stable"all is working
❯ DEPENDENCIES_NEXT=1 rc
Loading development environment (Rails 7.1.1)
development irb(main):001>
There was a problem hiding this comment.
working for us https://github.com/reidmorrison/rails_semantic_logger/pull/194/files#r1363832270
need to wait for next rails release that includes rails/rails#49621
This is merged |
|
We're running this branch ( @Amnesthesia @reidmorrison Are there further changes needed from you point of view or can this be merged and released? |
|
@severinkaelin Hey, nope — we're also running this in production without issues so far, just waiting for this to be merged! |
@Amnesthesia Great to hear! Thanks for confirming. @reidmorrison Is there an ETA for getting this PR merged and a release cut? Would be great to see this published to RubyGems. Just let me know if I can be of any help. Many thanks. |
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
`Logger#broadcast` was [removed in rails-7.1](reidmorrison#194), we should handle this
Issue # (if available)
Rails 7.1 dropped this method from ActiveSupport::Logger so trying to undefine it throws on boot
Description of changes
Removed that one undef line
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.