-
Notifications
You must be signed in to change notification settings - Fork 704
Upgrade Scala, JMockit version #4376
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
|
i will review and compile this PR, fix these issues which are very important for making new releases. |
|
Just i compiled this PR, got the below errors : [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.5.3:test (default-test) on project carbondata-common: The plugin org.apache.maven.plugins:maven-surefire-plugin:3.5.3 requires Maven version 3.6.3 -> |
will upgrade Maven version |
Yes, Maven version need to be above 3.6.3 since this PR upgrades the maven-surefire-plugin to 3.5.3 |
|
Successfully compiled. [INFO] Apache CarbonData :: Parent ........................ SUCCESS [ 1.101 s] |
| } | ||
|
|
||
| //@Test | ||
| public void testForNonDisributedSystem() { |
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.
why delete it?
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.
I think this testcase is not useful since it mocks up everything inside the testcase.
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
| return startBlockMinIsDefaultStart; | ||
| } | ||
|
|
||
| public boolean isRangeFullyCoverBlock() { |
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 add some description for public method
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.
fixed
| @SuppressWarnings("unused") | ||
| @Mock | ||
| long getMemorySize() { | ||
| return 15L; // 模拟 getMemorySize 方法 |
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 use english
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.
fixed
|
LGTM |
1 similar comment
|
LGTM |
|
LGTM+1 |
Why is this PR needed?
Scala and JMockit version is too low, out of support
What changes were proposed in this PR?
Scala, JMockit version is upgraded
Does this PR introduce any user interface change?
Is any new testcase added?