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

Support for compact prop in JSONFormatter #4

Merged
merged 7 commits into from Feb 11, 2020

Conversation

@dfurtado
Copy link
Contributor

dfurtado commented Feb 5, 2020

Adds support for compact properties in the JSONFormatter, this will replace the default properties "level", "timestamp" and "message" by property named using its first letter prefixed by @ symbol. For example: timestamp will be displayed as @t when Compact is set to true.

dfurtado added 2 commits Feb 5, 2020
Adds support for compact properties in the JSONFormatter, this will
replace the default properties "level", "timestamp" and "message" by
property named using its first letter prefixed by @ symbol. For example:
timestamp will be displayed as @t when Compact is set to true.
@dfurtado dfurtado requested a review from ConsulFirmin Feb 5, 2020
dfurtado added 4 commits Feb 5, 2020
Added test case for compact json format.
To avoid output logs to stdout when running the tests.
scopedLogger.go Outdated Show resolved Hide resolved
wlog.go Outdated Show resolved Hide resolved
Copy link
Member

ConsulFirmin left a comment

Left a couple of comments inline. Don't know if you can see them though.

One was regarding naming a mutex to mux (which is confusing. Mux stands for MUltipleXer)
The other was about sync.Mutex that needs no initialization

- changed the mutex is not initialized when creating a instance of
ScopedLogger.
- renamed "mux" to "mutex"
@dfurtado

This comment has been minimized.

Copy link
Contributor Author

dfurtado commented Feb 10, 2020

Thanks for the code review @ConsulFirmin I have pushed all the things that needed to be fixed.

@ConsulFirmin ConsulFirmin merged commit d55b5af into vargspjut:master Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.