-
Notifications
You must be signed in to change notification settings - Fork 94
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
Compatible old ReportUtil usage and supply test cases #14
Conversation
Pull Request Test Coverage Report for Build 52
💛 - Coveralls |
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.
Please change all the Chinese to English.
Please attach this PR to a milestone, and the PR description should be have more detail like this |
|
||
### Environment | ||
|
||
- SOFA Lookout version: |
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 NOT JUST COPY FROM OTHER PROJECT!
@@ -0,0 +1,40 @@ | |||
package com.alipay.sofa.common.utils; |
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 is the purpose of this test case? It seems that it is not related to this issue.
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.
#16 only litte cases and should provide more test cases after 。 PR additional remarks
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.
Please only do one thing in on PR next time.
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.
ok
* <p> | ||
* Created by yangguanchao on 18/06/04. | ||
*/ | ||
public class ReportUtil extends com.alipay.sofa.common.utils.ReportUtil{ |
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.
Add a Deprecated annotation here cause it is only for the compatible problem.
Motivation
Before some alipay users use
ReportUtil
with the full packagecom.alipay.sofa.common.log.ReportUtil
but in this versionReportUtil
has been moved tocom.alipay.sofa.common.utils.ReportUtil
. Consider many users has used, so we should be compatible with itModification
com.alipay.sofa.common.log.ReportUtil
extendscom.alipay.sofa.common.utils.ReportUtil
for compatibleResult
Fixes
#14
#16 only litte cases and after should provide more test cases