-
-
Notifications
You must be signed in to change notification settings - Fork 157
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
allow setting logLevel based on env variable #124
Comments
This is a good idea, I like it. My main concern is that it would be a breaking change, since users might be using that env var for something else, so this would suddenly change how their logging worked. We could somewhat mitigate this with a different variable name (e.g. Happy to take suggestions for how to avoid making this change require a major bump, but otherwise I'll definitely consider it for the future, and we should absolutely include it if/when we end up publishing loglevel 2.0. |
I am in favour of having Therefore, I agree with the point of not introducing this without some disclaimer as a breaking change. For the time being, I believe it's perfectly fine to have an alternative name as Later on, with the 2.0 migration notes, alert users of a possible name change for the env variable, if necessary. |
Is this similar to what this plugin does? https://github.com/vectrlabs/loglevel-debug As another suggestion, why not make the user of the environment variable opt-in? Users would have to configure the logger to use the environment variable, and could be configured on a module-wide setting (similar to |
Yes, very similar - that plugin looks like a great solution to this. Other than that, adding opt-in support in the short term isn't super useful imo. We could add a method to enable it, but it's super easy to opt-in by writing it from scratch anyway. The simplest fix is just: if (process.env.LOG_LEVEL != null) log.setLevel(process.env.LOG_LEVEL); That supports error/warn/info/etc values, or you could change it to We could create a Automatically picking the level from env vars would be definitely useful, but is a breaking change. For now I think I'm going to sit on this until I get around to the next breaking release, but it's definitely good fodder for that. |
Whatever name for env variable you choose, it should be possible to change the name of the variable (and disable it) so a user may avoid name conflicts is necessary. The name of the variable should not be However, it would require to implement some name hierarchy, e.g. dot Note: debug use semicolon and a star |
what about |
E.g. this |
It would be nice if we could have something like
LOG_LEVEL=1 node log-test.js
where ifLOG_LEVEL
(or any defined var name) could have an impact on the log level used rather than having to declare:This would be great for CI, in case we wanted to switch log levels without affecting the codebase.
The text was updated successfully, but these errors were encountered: