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

Reordered logging levels, added alert logger #105

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

anru
Copy link

@anru anru commented Apr 8, 2020

What's inside:

  1. Added the "alert" logger, it is intended for messages that must be seen, for example, to manually catch an error. This sense is near to old "debug" logger, if you want use them.
  2. The "debug" level now has a low priority (but still enabled by default) and is not so visually highlighted: it is intended for unimportant messages that are usually not needed, but which would be useful to have in the system if something goes wrong.
  3. In case someone wants to inherit from the "Signale" class, the "scope" method now correctly creates an instance of the current class with which it was created.
  4. Added the ability to set your own log levels, without having to inherit to override the "_logLevels" getter.

Fixes #96

What's inside:

1. Added the "alert" logger, it is intended for messages that must be seen, for example, to manually catch an error.
2. The "debug" level now has a low priority and is not so visually highlighted: it is intended for unimportant messages that are usually not needed, but which would be useful to have in the system if something goes wrong.
3. In case someone wants to inherit from the "Signale" class, the "scope" method now correctly creates an instance of the current class with which it was created.
4. Added the ability to set your own log levels, without having to inherit to override the "_logLevels" getter.

Fixes klaudiosinani#96
@anru
Copy link
Author

anru commented Apr 21, 2020

Ok, if someone wanted this PR merged right now for production use I made a fork and published npm package signales with #105 PR merged into it.

@b4nst
Copy link

b4nst commented Apr 21, 2020

I find that move kind of harsh. I agree with you on the content. It seems that this repo is going on the dark path of inactivity, and this is really frustrating since there is some works pending.

But I don't think this is the good solution. If everybody starts to fork and release their own version of the application, it will be a mess. And this is an heavy anti-pattern of Open Source. We need an issue to discuss about the inactivity and the right solutions to deal with it.

On an other point, your PR is not mergeable :

  1. You didn't write any test. How are we supposed to say if it's correct or not ? Try to think of other people that will deal with your code in the future.
  2. I already did a PR about level reordering, if you want to add alert level, this is another pr. If you have some doubts about my work, let's talk about it on the existing PR, I'm totally open to critics. I find your action disrespectful against my work and the community.

I'm sad to see this selfishness in the open source world.

@anru
Copy link
Author

anru commented Apr 27, 2020

Added two fixes to PR:

  • logLevel now is debug by default (so, no one debug output will be lost)
  • GitHub checks are happy now: lock "xo" version to "0.24.0" in order to have reproducible checks

@klaudiosinani klaudiosinani added the enhancement New feature or request label Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reorder logging levels to reduce debug noise
3 participants