-
Notifications
You must be signed in to change notification settings - Fork 19
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
Move standard formatters to public export #34
Comments
Hey Jonathan! Killer suggestion. Bristol has a stated goal of lazy-loading everything possible, so that leaves us with two options toward that goal:
I like 2 for its simplicity, but 1 gives you that peace-of-mind guarantee since it's in the code. Any thoughts? :) Also: Perhaps a better option here would be to make it possible to set "globals" just on one particular target? I'm working on a revamp of how targets are stored and configured, so this could be a very easy feature add if there aren't a bunch of other use cases where exposing the built-in formatters is still the better option. |
I also modified the JSON formatter for my use and I created PR #22 so I didn't have to duplicate the Bristol helper code. I then just created my own JSON formatter based on Bristol's and used the original Bristol utils. This worked out well for me and I don't have to rely on paths and I don't have to monkey patch JSON formatter code. |
Hi @TomFrost and @MarkHerhold 👋, I don't mind requiring the formatter directly, and its pretty standard practice that I see around the community. I'm totally ok with option 2 if you would consider those to be versioned paths 👍 As far as "globals" per targets are concerned -- I think that would be a great feature because most of the time one log format requires some static attributes ( Looking at you Logstash ) than other simpler targets where those globals are just noise. |
Hi @TomFrost and contributors,
Would you be open to moving including the formatters included with the module the top level export ? One example for the reasoning behind this request:
The json formatter is almost 100% what I want for everything, but I'd like to add static properties to a particular target ( not globally ). To achieve that, I can modify the function arguments before invoking the standard json function.
I currently can do this just by just reaching into the module (
require('bristol/lib/formatters/json')
) but its brittle because you can change that path an any time 😄Thanks for the module 👍
The text was updated successfully, but these errors were encountered: