-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Streamline logging #71
Conversation
@@ -293,7 +294,7 @@ public BasePanel(JabRefFrame frame, BibtexDatabase db, File file, | |||
fileMonitorHandle = Globals.fileUpdateMonitor.addUpdateListener(this, | |||
file); | |||
} catch (IOException ex) { | |||
BasePanel.logger.warning(ex.toString()); | |||
LOGGER.debug("Could not register FileUpdateMonitor", ex); |
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.
shouldn't this be LOGGER.warn
instead of LOGGER.debug
?
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.
The log levels and practices of commons.logging and java.util.logging are somewhat different. Since I converted the whole thing to commons, I tried to follow their guidelines. In many cases, such as the above, it is quite hard for me to decide the criticality of the event, and, hence, the log level. If you know that this event is really that critical, it is no problem to change the logging. Should I do that? Perhaps ``LOGGER.error` would also be appropriate?
And add a few serialVersionUIDs on the fly :)
… is needed there before initalizing Globals.
This pull request streamlines the usage of logging APIs in JabRef. All direct usages of
java.util.logging
are now replaced by usages ofcommons.logging
. Internally,commons.logging
is configured to default tojava.util.logging
, so nothing is fundamentally changed.java.util.logging
is still needed in a single class,CacheableHandler
, to access the actual log for display in the UI. This is not possible directly withcommons.logging
and requires the usage of the underlying logging API.The major benefits are of this pull request are: