-
Notifications
You must be signed in to change notification settings - Fork 8
Added log message about deprecated method usage. #373
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
public class AnalyticsService { | ||
|
||
public void trackConfigChange(String instanceId, Map<String, Object> configDetail) { | ||
//no need to implement |
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.
Do we need this 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.
This is part of codeSnippet i.e. AnalyticsService is used in MonitoringLogger that will be presented in logs.
I added this comment to indicate that this is conscious decision to left implementation empty.
What would you do in such case ? Remove comment?
import com.pubnub.api.logging.LogMessageContent; | ||
import com.pubnub.api.logging.LogMessageType; | ||
|
||
public class MonitoringLogger implements CustomLogger { |
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.
What's the purpose of this 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.
This is code snippet that will be used in docs
timestamp: String, | ||
location: String? | ||
) { | ||
// no need to implement |
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 same here, do we need thee // no need to implement
comments?
} | ||
|
||
override fun debug(message: LogMessage) { | ||
// todo remove |
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.
Should it be removed?
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 catch. I will remove it.
No description provided.