-
Notifications
You must be signed in to change notification settings - Fork 250
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
[Core][Logger] Add option to output a time stamp at the beginning of each log line #5676
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not agree, why not using the timer that already exist in kratos?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments
|
I said that before check the code, sorry I thought this was a timer 2.0, nevermind |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the feature and think that it is really useful. However, I have some comments about how it is done:
- Please note that the message has a
mTime
member which I would initialize it in the constructor of the message. So we would get the same timestamp for the same message in all outputs. - I would make the LoggerOutput in charge of timestamp formatting. This make it customizable f like other output formatting and not fixed by the unique message class.
- Personally I like more a dense YYYYMMDDHHmmss format which is normally used in log files. Of course, this is my personal opinion and I am open to hear others on it.
Similar to the severity level, a flag for adding the time stamp at the beginning of each log line can be set to the Logger.
e.g.
This can be activated by:
@KratosMultiphysics/technical-committee