-
Notifications
You must be signed in to change notification settings - Fork 172
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
Please support local timezone #7
Comments
Second this. Timestamp in local time zone will be convenient. It will be even better if the logger can take TZ environment variable. typos:
suggestion:
I have looked at many logging libraries online. I was about to give up to write my own and then I found quill:) This library looks really promising. Great work! Thank you! |
Hello, Thanks for the good feedback and the comments
With regards to DEBUG vs LOG_DEBUG. The reason for using 'LOG_DEBUG' is that in a large application in past we found out that the keyword 'DEBUG' could be printed also by other libraries. Therefore, we wanted to be able to filter out the debug logs only and a good way was by using 'LOG_DEBUG'. To keep everyone happy I think those predefined strings can be moved to tweakme.h where each user can customise them based on their needs |
hi,
Now I know https://github.com/odygrd/quill/blob/master/examples is a good place for documentation. But it is not so obvious by just reading README.
|
Regarding 'DEBUG' vs 'LOG_DEBUG', for users who need 'LOG_DEBUG' to filter, they can do If the above line works, then it is up to the user how to make log level string unique for filtering. It can be "DEBUG", "[DEBUG]", or "LOG_DEBUG", depending on various application needs. |
Thanks a lot for feedback and ideas. |
If there are no more requests in this, I am closing it tomorrow |
It looks really good! Thanks! |
FYI. I tried to build quill 28a4575 with my existing fmt 6.1.2, got a lot of compile errors about ostream.h:120, type_identity_t was not declared in this scope. changed my fmt to 6.2.0 and everything worked.
|
A workaround for fmt 6.1.2: include quill.h before 6.1.2 fmt/format.h. |
Now quill requires users to have the fmt 6.2.0, which seems to have some backward compatibility issue. Is every user willing to upgrade to this version? Is it possible/better to let users specify where fmt installation is, instead of always using the bundled fmt headers? It is your call. I have no issue to use the latest fmt version as my app is pretty new, not in production yet. |
I see ... :( I tried a few things like putting the bundled fmt under separate namespace etc but it won't work because of all the preprocessor definitions when the fmt 6.1.2 header is included If your app is new one solution would be instead of having your own lib fmt version to use the bundled one from Quill directly, like include To support different fmt versions the only solution looks like supporting an FMT_EXTERNAL cmake option where Quill uses the external fmt library instead of the bundled one |
Closing this and moving it to #11 |
Hey @azuresigma Could you please try with b66fd65 and confirm. Documentation is here will add to the README later.. I hope it resolves the issue of using an external version of lib fmt! |
Hi, sorry I didn't see your comment here. I did see your commit b66fd65 yesterday and tried it. It worked! I used 'cmake -DCMAKE_PREFIX_PATH=/my/fmt/fmt-config.cmake-directory/ -DQUILL_FMT_EXTERNAL=ON -DCMAKE_INSTALL_PREFIX=/new/quill/install-dir/'. Make output showed bundled fmt cc files were not compiled. The app program builds and works as expected. Thank you for the great work! |
Thank you for your great work!
I love this library.
One thing to prevent using quill is supporting local time zone.
Please support local timezone.
Currently quill::PatternFormatter::_timezone_type is private and hard coded.
The text was updated successfully, but these errors were encountered: