Skip to content

[Dubbo-3886] Integrate with SOFARegistry.#3886

Merged
lovepoem merged 9 commits intoapache:masterfrom
ujjboy:integrate_with_sofa_registry
May 2, 2019
Merged

[Dubbo-3886] Integrate with SOFARegistry.#3886
lovepoem merged 9 commits intoapache:masterfrom
ujjboy:integrate_with_sofa_registry

Conversation

@ujjboy
Copy link
Copy Markdown
Member

@ujjboy ujjboy commented Apr 17, 2019

What is the purpose of the change

#3874

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

  • Make sure there is a GITHUB_issue field 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 [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. 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 clean install -DskipTests=false & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
  • If this contribution is large, please follow the Software Donation Guide.

@ujjboy ujjboy changed the title [WIP] Integrate with SOFARegistry. [Dubbo-3886] Integrate with SOFARegistry. Apr 17, 2019
@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 17, 2019

Codecov Report

Merging #3886 into master will increase coverage by 0.1%.
The diff coverage is 85.34%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #3886     +/-   ##
===========================================
+ Coverage     63.85%   63.95%   +0.1%     
  Complexity       98       98             
===========================================
  Files           715      718      +3     
  Lines         31457    31613    +156     
  Branches       5049     5090     +41     
===========================================
+ Hits          20086    20218    +132     
- Misses         9061     9078     +17     
- Partials       2310     2317      +7
Impacted Files Coverage Δ Complexity Δ
...che/dubbo/registry/sofa/SofaRegistryConstants.java 0% <0%> (ø) 0 <0> (?)
...pache/dubbo/registry/sofa/SofaRegistryFactory.java 100% <100%> (ø) 0 <0> (?)
...a/org/apache/dubbo/registry/sofa/SofaRegistry.java 85.58% <85.58%> (ø) 0 <0> (?)
...che/dubbo/remoting/transport/mina/MinaChannel.java 43.42% <0%> (-10.53%) 0% <0%> (ø)
...dubbo/common/status/support/LoadStatusChecker.java 46.15% <0%> (-3.85%) 0% <0%> (ø)
.../dubbo/monitor/support/AbstractMonitorFactory.java 78.37% <0%> (-2.71%) 0% <0%> (ø)
...ting/exchange/support/header/HeartbeatHandler.java 83.72% <0%> (-2%) 0% <0%> (ø)
.../remoting/transport/netty4/NettyClientHandler.java 68.42% <0%> (-1.23%) 0% <0%> (ø)
...he/dubbo/remoting/transport/netty/NettyClient.java 66.1% <0%> (-1.14%) 0% <0%> (ø)
...e/dubbo/remoting/transport/netty4/NettyClient.java 70.58% <0%> (-1.06%) 0% <0%> (ø)
... and 32 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e189048...6e68107. Read the comment docs.

@ujjboy
Copy link
Copy Markdown
Member Author

ujjboy commented Apr 22, 2019

@ralf0131 Please review this PR.

@ralf0131
Copy link
Copy Markdown
Contributor

I will try to do it today or tomorrow..

Copy link
Copy Markdown
Contributor

@ralf0131 ralf0131 left a comment

Choose a reason for hiding this comment

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

Please following the guide to add a new module: apache/dubbo-website#350

There are some things to do:

  • Add your dependency to dubbo-bom/pom.xml
  • Add your dependency to dubbo-distribution/pom.xml
  • Update dubbo-dependencies if necessary
  • Could you list the license of your dependencies to make sure they are compatible with Apache

Comment thread dubbo-all/pom.xml Outdated
Comment thread dubbo-all/pom.xml Outdated
Comment thread dubbo-registry/dubbo-registry-sofa/pom.xml
Comment thread dubbo-registry/dubbo-registry-sofa/pom.xml
Comment thread dubbo-rpc/dubbo-rpc-thrift/pom.xml Outdated
Comment thread dubbo-dependencies-bom/pom.xml Outdated
Comment thread dubbo-dependencies-bom/pom.xml Outdated
Copy link
Copy Markdown
Member

@lovepoem lovepoem left a comment

Choose a reason for hiding this comment

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

Please use junit5

Comment thread dubbo-config/dubbo-config-spring/pom.xml
@ujjboy ujjboy force-pushed the integrate_with_sofa_registry branch from b5a6d77 to b5f8218 Compare April 26, 2019 12:17
@ujjboy ujjboy force-pushed the integrate_with_sofa_registry branch from 0b1ac21 to 3e6157c Compare April 27, 2019 12:05
@ujjboy
Copy link
Copy Markdown
Member Author

ujjboy commented Apr 27, 2019

@ralf0131 @lovepoem Hi, I have changed the code according to review, but CI only build error in openjdk11, Can you retry it?

Copy link
Copy Markdown
Contributor

@ralf0131 ralf0131 left a comment

Choose a reason for hiding this comment

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

LGTM.

@ralf0131
Copy link
Copy Markdown
Contributor

@lovepoem What is you opinion on this?

@ralf0131
Copy link
Copy Markdown
Contributor

I checked the license of dubbo-registry-sofa dependencies. All looks good.

@lovepoem
Copy link
Copy Markdown
Member

lovepoem commented May 2, 2019

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants