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
[Improvement][server] server model ProcessUtils.java code cleaning. #3382
[Improvement][server] server model ProcessUtils.java code cleaning. #3382
Conversation
*/ | ||
public static void cancelApplication(List<String> appIds, Logger logger, String tenantCode,String executePath) | ||
throws IOException { | ||
public static void cancelApplication(List<String> appIds, Logger logger, String tenantCode,String executePath) { |
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.
public static void cancelApplication(List<String> appIds, Logger logger, String tenantCode,String executePath) { | |
public static void cancelApplication(List<String> appIds, Logger logger, String tenantCode, String executePath) { |
Hi,
Please configure the checkstyle.xml and codestyle plugin to your ide. It will help to format the code.
[1] https://github.com/apache/incubator-dolphinscheduler/blob/dev/style/checkstyle.xml
[2] https://github.com/apache/incubator-dolphinscheduler/blob/dev/style/intellij-java-code-style.xml
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,I'll do it. I'll re submit the code
@@ -318,7 +308,7 @@ public static void cancelApplication(List<String> appIds, Logger logger, String | |||
|
|||
Runtime.getRuntime().exec(runCmd); | |||
} catch (Exception e) { | |||
logger.error("kill application error", e); | |||
logger.error("kill application error : {}",e.getMessage(),e); |
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.
There is no necessary to change this.
If we use
logger.error("kill application error", e);
actually, it is the function of [1], and it will show enough info of this error.
If we use
logger.error("kill application error : {}",e.getMessage(),e);
it will be the function[2]. So we do not need to change this.
[1]
[2]
* | ||
* @param processId process id | ||
* @return pids | ||
* @throws Exception exception | ||
*/ | ||
public static String getPidsStr(int processId)throws Exception{ | ||
public static String getPidsStr(int processId)throws Exception { |
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.
public static String getPidsStr(int processId)throws Exception { | |
public static String getPidsStr(int processId) throws Exception { |
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,I'll change it. At present, ckckstyle has not been detected. I hope it can be added.
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,I'll change it. At present, ckckstyle has not been detected. I hope it can be added.
Ok, Thx for your reminding~
Kudos, SonarCloud Quality Gate passed! 0 Bugs |
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.
LGTM.
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.
+1
What is the purpose of the pull request
server model ProccessUtils.java code cleaning.
Brief change log
1.Remove unreferenced variables
2.Remove unnecessary throws exception type.
Verify this pull request
This pull request is code cleanup without any test coverage.