-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
log version number in each log line #1562
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
Conversation
|
@MorrisJobke, thanks for your PR! By analyzing the annotation information on this pull request, we identified @LukasReschke, @bartv2 and @MTGap to be potential reviewers |
82a22f7 to
831b6af
Compare
LukasReschke
left a comment
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.
Good stuff :)
|
LGTM |
|
Personally, I don't like it, it's an excuse for doing proper bug reports. If version is missing, then other necessary information is probably missing, too, and you'd need to ask for other stuff anyway. ±0 |
It's also helpful in case you want to quickly take a look in your log monitoring systems when an error appeared and on what versions :) … not really super awesome to always have to look up when one updated etc… |
|
I can see the point, but I don't really like to invoke yet another class when writing a log. |
IConfig is already used in that function, maybe manually read from the config version? |
I also checked it and all it does it reads the current session. And the version is loaded anyways before. I feared also to add another dependency there, but it would really help to make looking at logs easy - especially when doing upgrade testing. |
|
@blizzz @nickvergessen Yes or no? I would love to see this included. |
|
Maybe others also have an opinion on this? @schiessle @rullzer @icewind1991 |
|
I'm okay, when we use |
Signed-off-by: Morris Jobke <[email protected]>
831b6af to
0eb9920
Compare
Done :) |
|
👍 |
Makes it a lot easier to analyze log files, because then you see if the log was fixed with a recent version or was caused by a recent version for example.
cc @nickvergessen @LukasReschke @schiessle @blizzz