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

Replace Kernel warnings with Listen.logger #574

Closed
wants to merge 2 commits into from

Conversation

AlexB52
Copy link
Contributor

@AlexB52 AlexB52 commented Nov 29, 2023

Fix #572

In case we can move these calls to Listen.logger here is the PR for the change.

Some warning messages cannot be silenced by choosing a logger level. This change
standardizes warning messages using the Listen.logger
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` paramete...
Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` parameter (in .rubocop.yml).
Supported versions: 2.4, 2.5, 2.6, 2.7, 2.8

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` paramete...
Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` parameter (in .rubocop.yml).
Supported versions: 2.4, 2.5, 2.6, 2.7, 2.8

Since Listen.logger.warn formats the error message with a `WARN -- : #{msg}`
we don't need the [Listen warning] part as it is redundant
Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

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

Some files could not be reviewed due to errors:

Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` paramete...
Error: RuboCop found unknown Ruby version 3.0 in `TargetRubyVersion` parameter (in .rubocop.yml).
Supported versions: 2.4, 2.5, 2.6, 2.7, 2.8

@ColinDKelley
Copy link
Collaborator

@AlexB52 I'm concerned that log warnings will almost never be seen. That's why Kernel.warn or Kernel#warn was being called--so that these warnings would be visible. But clearly there are cases where people don't want to see one or more of the warnings.

I wonder if instead the warnings could be configurable in listen? Maybe there could be a proc that could be registered that would get to look at each warning and decide whether to show it or not?

@AlexB52
Copy link
Contributor Author

AlexB52 commented Feb 22, 2024

@ColinDKelley
Not sure I understand the registration proc. Is your idea of a proc like this?
What is the preferred way to make this globally available?
Can you provide some pseudocode to illustrate the idea please?

warn = ->(message) {
  return if ENV['LISTEN_GEM_DISABLE_WARNING'].present?

  Kernel.warn(message)
}

What about a Listen::Warning class that calls Kernel.warn unless a ENV variable LISTEN_GEM_DISABLE_WARNING is set. That would make it a bit more explicit maybe?

Something like:

module Listen
  class Warning
    module_function
    def warn(message)
      return if ENV['LISTEN_GEM_DISABLE_WARNING'].present?

      Kernel.warn(message)
    end
  end
end

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.

Move #warn calls from Kernel to Logger
3 participants