-
Notifications
You must be signed in to change notification settings - Fork 298
Removing Deprecated LogWrapper API #154
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
| @Override | ||
| public void onKill(String reason) { | ||
| if (logger.logsDebug()) { | ||
| if (logger.isDebugEnabled()) { |
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.
You should be able to remove this check here as well with minimal performance penalty.
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.
Done
| if (logger.logsDebug()) { | ||
| logger.debug("removing query " + query); | ||
| } | ||
| logger.debug("removing query {}", query); |
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.
Label missing.
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.
Done
| private static final String TRANSACTION_TOO_MANY_RETRIES = "maxretries"; | ||
| private static final String TRANSACTION_OVERRIDE_BY_SET = "overriddenBySet"; | ||
|
|
||
| private static final Logger operationLogger = LoggerFactory.getLogger(Repo.class); |
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.
You are now using this in places were you previously used 'transactionLogger'. I'd suggest naming this just 'logger'.
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.
Done
* Adding the sync APIs for FirebaseAuth * Added more tests; Added sync APIs for FirebaseMessaging * Removing Task references from database, iid and fcm APIs * Fixing a typo * Minor code clean up * Updated javadocs; Renamed internal helpers of FirebaseMessaging for consistency * Removed the deprecated FirebaseCredential API (#149) * Removing the Task API (#152) * Removed the deprecated FirebaseCredential API * Removing the deprecated Task API * Dropping Support for App Engine Java 7 Runtime (#153) * Dropping support for GAE 7 * Removed GaeThreadFactory, GaeExecutorService and RevivingScheduledExecutor * Removed the deprecated FirebaseCredential API * Removing GAE java7 related APIs (GaeThreadFactory, RevivingScheduledExecutor) * Removed GaePlatform implementation * Added FirebaseScheduledExecutor * Updated documentation * Some minor nits from code reviews * Calling super method in DefaultRunLoop executor * Removing Deprecated LogWrapper API (#154) * Dropping support for GAE 7 * Removed GaeThreadFactory, GaeExecutorService and RevivingScheduledExecutor * Removed the deprecated FirebaseCredential API * Removing GAE java7 related APIs (GaeThreadFactory, RevivingScheduledExecutor) * Removed GaePlatform implementation * Added FirebaseScheduledExecutor * Updated documentation * Removing LogWrapper API * Removing PrefixedLogger * Removing test config file * Updated CHANGELOG * Minor clean ups pointed out in the code review * Minor API doc fixes (#171)
Removing the deprecated
com.google.firebase.database.loggingpackage, and using SLF4J as the primary logging facade.Some sample debug logs obtained with SLF4J and
java.util.logging:Same with the SLF4J simple logger: