-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
more code review. #3083
more code review. #3083
Conversation
dubbo-common/src/main/java/org/apache/dubbo/common/utils/UrlUtils.java
Outdated
Show resolved
Hide resolved
dubbo-common/src/main/java/org/apache/dubbo/common/utils/UrlUtils.java
Outdated
Show resolved
Hide resolved
dubbo-common/src/main/java/org/apache/dubbo/common/utils/UrlUtils.java
Outdated
Show resolved
Hide resolved
dubbo-common/src/main/java/org/apache/dubbo/common/utils/UrlUtils.java
Outdated
Show resolved
Hide resolved
...ubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryDirectory.java
Show resolved
Hide resolved
...ubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryDirectory.java
Show resolved
Hide resolved
...ubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryDirectory.java
Show resolved
Hide resolved
Will you please take a look at the suggestions from @khanimteyaz. Also, notice that the Travis CI has passed in master, so please rebase master branch and make sure your PR passes the CI check. |
Codecov Report
@@ Coverage Diff @@
## master #3083 +/- ##
============================================
- Coverage 63.43% 63.42% -0.01%
Complexity 75 75
============================================
Files 653 653
Lines 28261 28228 -33
Branches 4820 4807 -13
============================================
- Hits 17926 17904 -22
+ Misses 8057 8042 -15
- Partials 2278 2282 +4
Continue to review full report at Codecov.
|
@beiwei30 do let me know once you are done with your review comments. I will try to give fast-fwd look 😄 |
I am about to give some integration tests on the master branch, this patch seems to work fine. @beiwei30 If you have other improvement based on the review, please go ahead with a new PR. |
@khanimteyaz feel free to submit pull request if you need to refactor further. |
Sure.
…On Sat, Dec 29, 2018 at 1:52 PM Ian Luo ***@***.***> wrote:
@khanimteyaz <https://github.com/khanimteyaz> feel free to submit pull
request if you need to refactor further.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3083 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AdXcPR-vj-ItH4Vp2TuKDVYXTD2ePNcaks5u9yYqgaJpZM4Ziqq3>
.
|
…view. * refactor ScriptRouter * refactor TagRouter * refactor AbstractConfiguratorListener * make sure parameter should not be null * correct comments * make ReferenceConfigurationListener private static * avoid dup code in init * add fixme for potential useless code * clean up useless variables * move methods into UrlUtils * make method private * reformat javadoc * avoid dup code * reformat log message * reformat log message * reformat the code * remove useless imports * remove useless code * refactor ScriptRouter * refactor TagRouter * refactor AbstractConfiguratorListener * Add comment * Fix UT * make sure parameter should not be null * correct comments * make ReferenceConfigurationListener private static * Revert demo changes * Revert code to avoid NPE in RPC wire after providers are cleared. * make ListenableRouter code thread safe * Fix UT * Remove assert check to continue with execute. * avoid dup code in init * solve compile error * add fixme for potential useless code * clean up useless variables * move methods into UrlUtils * make method private * reformat javadoc * avoid dup code * reformat log message * reformat log message * reformat the code * remove useless imports * remove useless code * code review comments from @khanimteyaz * code review from @khanimteyaz
What is the purpose of the change
XXXXX
Brief changelog
XXXXX
Verifying this change
XXXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[Dubbo-XXX] Fix UnknownException when host config not exist #XXX
. Each commit in the pull request should have a meaningful subject line and body.mvn clean install -DskipTests=false
&mvn clean test-compile failsafe:integration-test
to make sure unit-test and integration-test pass.