Skip to content
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

Remove dependency of apache-common-lang3 in the client. #3031

Merged
merged 8 commits into from
Jun 15, 2020

Conversation

zongtanghu
Copy link
Collaborator

Please do not create a Pull Request without creating an issue first.

What is the purpose of the change

Remove dependency of apache-common-lang3 in the client, such as replacing NumberUtils with ConvertUtils in all modules of nacos.

Brief changelog

The main goal is to remove dependency for apache commons-lang package.

Verifying this change

I have already paased the unit test cases.

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

@zongtanghu
Copy link
Collaborator Author

Hi @yanlinly @chuntaojun please help to review this pr, thanks.

@yanlinly
Copy link
Collaborator

#2952 is same?

@zongtanghu
Copy link
Collaborator Author

I have just removed the apache common-lang3 package in the client. @yanlinly

@@ -52,7 +59,7 @@ public FailoverReactor(HostReactor hostReactor, String cacheDir) {
this.hostReactor = hostReactor;
this.failoverDir = cacheDir + "/failover";
// init executorService
this.executorService = Executors.newSingleThreadScheduledExecutor(new ThreadFactory() {
this.executorService = new ScheduledThreadPoolExecutor(1, new ThreadFactory() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里改的原因是?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是PMD报出来的问题。。。 所以我就顺手修改了

@zongtanghu zongtanghu mentioned this pull request Jun 11, 2020
5 tasks
/**
* @author <a href="mailto:[email protected]">liaochuntao</a>
*/
public final class ConvertUtils {

private static final String NULL_STR = "null";

public static int toInt(String val) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may need to add a comment, which automatically defaults to 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I will add a comment in next commit.

* @param str the String to check; upper and lower case are treated as the same
* @return the Boolean value of the string, {@code null} if no match or {@code null} input
*/
@SuppressWarnings({"PMD.UndefineMagicConstantRule", "PMD.AvoidComplexConditionRule", "PMD.SwitchStatementRule"})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

直接类上面,@SuppressWarnings(“all”)吧

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I will adjust this codes in next commit.

/**
* @author zongtanghu
*/
public class CharSequenceUtils {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不必要单独创建一个新的类,这个放到StringUtils内部就好了

@KomachiSion KomachiSion linked an issue Jun 12, 2020 that may be closed by this pull request
@zongtanghu
Copy link
Collaborator Author

There is one ci test case failed, because it executed time out. @yanlinly @chuntaojun Can you help to rerun the ci job again?

@CLAassistant
Copy link

CLAassistant commented Jun 12, 2020

CLA assistant check
All committers have signed the CLA.

zongtanghu added a commit to zongtanghu/nacos that referenced this pull request Jun 13, 2020
@KomachiSion
Copy link
Collaborator

Please fix confilcts.

@KomachiSion KomachiSion added the status/merge-conflict Category prs with merge conflict. label Jun 15, 2020
@zongtanghu
Copy link
Collaborator Author

Please fix confilcts.

Okay, I will fix the codes' conflicts right now. @KomachiSion

@zongtanghu
Copy link
Collaborator Author

@yanlinly @KomachiSion @chuntaojun I have already done the codes' conflicts.

import java.util.concurrent.*;


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove extra blank line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the next commit, I will remove the blank line.

zongtanghu added a commit to zongtanghu/nacos that referenced this pull request Jun 15, 2020
}
}

// end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些代码是从哪里拷贝的,还是自己写的呢? 如果是拷贝过来的注意一下license


}

// end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些代码是从哪里拷贝的,还是自己写的呢? 如果是拷贝过来的注意一下license

yanlinly
yanlinly previously approved these changes Jun 15, 2020
Copy link
Collaborator

@yanlinly yanlinly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@KomachiSion KomachiSion added dependencies Pull requests that update a dependency file and removed status/merge-conflict Category prs with merge conflict. labels Jun 15, 2020
@KomachiSion KomachiSion self-requested a review June 15, 2020 05:39
@yanlinly yanlinly merged commit 3f0023a into alibaba:develop Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove dependency of apache-common-lang3 in the client.
5 participants