You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@dubbo.apache.org by "wanghbxxxx (GitHub)" <gi...@apache.org> on 2019/02/16 06:00:43 UTC

[GitHub] [incubator-dubbo] wanghbxxxx opened pull request #3489: refactor generateExtNameAssignment to reduce complexity

## 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:

- [x] Make sure there is a [GITHUB_issue](https://github.com/apache/incubator-dubbo/issues) 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](https://github.com/apache/incubator-dubbo/tree/master/dubbo-test).
- [ ] 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](https://github.com/apache/incubator-dubbo/wiki/Software-donation-guide).


[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3489 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] wanghbxxxx commented on pull request #3489: refactor generateExtNameAssignment to reduce complexity

Posted by "wanghbxxxx (GitHub)" <gi...@apache.org>.

thanks

[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3489 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] wanghbxxxx commented on issue #3489: refactor generateExtNameAssignment to reduce complexity

Posted by "wanghbxxxx (GitHub)" <gi...@apache.org>.

> I believe UT are verified, if not or UT not present could you plz add those. Thanks for simplifying the implementation 👍

actually there is no UT at all, it is covered by UT of ExtensionLoader.
i'd like to add some UT which is not easy, because there is only one public method which does a lot.
maybe i could make some of private methods public/protected which can be mocked.
or define a java bean contains class info like classname, implemeted interfaces,
which can be test easily
 could you give me some advice please!
thank you

[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3489 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3489: refactor generateExtNameAssignment to reduce complexity

Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.

I believe UT are verified, if not or UT not present could you plz add those. Thanks for simplifying the implementation 👍 

[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3489 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] khanimteyaz commented on pull request #3489: refactor generateExtNameAssignment to reduce complexity

Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.

use dubbo `Constants.PROTOCOL_KEY` instead  **protocol**

[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3489 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] wanghbxxxx commented on issue #3489: refactor generateExtNameAssignment to reduce complexity

Posted by "wanghbxxxx (GitHub)" <gi...@apache.org>.

> I believe UT are verified, if not or UT not present could you plz add those. Thanks for simplifying the implementation 👍

actually there is no UT at all, it is covered by UT of ExtensionLoader.
i'd like to add some UT which is not easy, because there is only one public method which does a lot.
maybe i could make some of private methods public/protected.
could you give me some advice please!
thank you

[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3489 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] codecov-io commented on issue #3489: refactor generateExtNameAssignment to reduce complexity

Posted by "codecov-io (GitHub)" <gi...@apache.org>.

# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=h1) Report
> Merging [#3489](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/d33784d3c887aefd647e24a9e46f58be8ad4f02a?src=pr&el=desc) will **decrease** coverage by `0.07%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/graphs/tree.svg?width=650&token=VnEIkiFQT0&height=150&src=pr)](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##             master    #3489      +/-   ##
============================================
- Coverage      64.1%   64.03%   -0.08%     
  Complexity       71       71              
============================================
  Files           664      664              
  Lines         28605    28593      -12     
  Branches       4769     4763       -6     
============================================
- Hits          18338    18310      -28     
- Misses         8060     8076      +16     
  Partials       2207     2207
```


| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...o/common/extension/AdaptiveClassCodeGenerator.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vZXh0ZW5zaW9uL0FkYXB0aXZlQ2xhc3NDb2RlR2VuZXJhdG9yLmphdmE=) | `98.09% <100%> (+3.22%)` | `0 <0> (ø)` | :arrow_down: |
| [.../remoting/transport/netty4/NettyServerHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5U2VydmVySGFuZGxlci5qYXZh) | `61.9% <0%> (-9.53%)` | `0% <0%> (ø)` | |
| [...apache/dubbo/common/config/ConfigurationUtils.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vY29uZmlnL0NvbmZpZ3VyYXRpb25VdGlscy5qYXZh) | `40% <0%> (-8%)` | `0% <0%> (ø)` | |
| [...ava/org/apache/dubbo/config/DubboShutdownHook.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9EdWJib1NodXRkb3duSG9vay5qYXZh) | `75% <0%> (-6.25%)` | `0% <0%> (ø)` | |
| [...ng/transport/dispatcher/all/AllChannelHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvZGlzcGF0Y2hlci9hbGwvQWxsQ2hhbm5lbEhhbmRsZXIuamF2YQ==) | `51.42% <0%> (-5.72%)` | `0% <0%> (ø)` | |
| [.../org/apache/dubbo/remoting/ExecutionException.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9FeGVjdXRpb25FeGNlcHRpb24uamF2YQ==) | `15.78% <0%> (-5.27%)` | `0% <0%> (ø)` | |
| [...onfig/spring/extension/SpringExtensionFactory.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvZXh0ZW5zaW9uL1NwcmluZ0V4dGVuc2lvbkZhY3RvcnkuamF2YQ==) | `80.48% <0%> (-4.88%)` | `0% <0%> (ø)` | |
| [...e/dubbo/remoting/transport/netty/NettyChannel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JlbW90aW5nL3RyYW5zcG9ydC9uZXR0eS9OZXR0eUNoYW5uZWwuamF2YQ==) | `57.64% <0%> (-4.71%)` | `0% <0%> (ø)` | |
| [...c/main/java/org/apache/dubbo/rpc/RpcException.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9ScGNFeGNlcHRpb24uamF2YQ==) | `82.75% <0%> (-3.45%)` | `0% <0%> (ø)` | |
| [...exchange/support/header/HeaderExchangeHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9leGNoYW5nZS9zdXBwb3J0L2hlYWRlci9IZWFkZXJFeGNoYW5nZUhhbmRsZXIuamF2YQ==) | `55.81% <0%> (-2.33%)` | `0% <0%> (ø)` | |
| ... and [4 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=footer). Last update [d33784d...8cd8d10](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3489 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] codecov-io commented on issue #3489: refactor generateExtNameAssignment to reduce complexity

Posted by "codecov-io (GitHub)" <gi...@apache.org>.

# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=h1) Report
> Merging [#3489](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/d33784d3c887aefd647e24a9e46f58be8ad4f02a?src=pr&el=desc) will **decrease** coverage by `0.14%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/graphs/tree.svg?width=650&token=VnEIkiFQT0&height=150&src=pr)](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##             master    #3489      +/-   ##
============================================
- Coverage     64.12%   63.98%   -0.15%     
  Complexity       71       71              
============================================
  Files           664      664              
  Lines         28605    28592      -13     
  Branches       4769     4762       -7     
============================================
- Hits          18343    18294      -49     
- Misses         8056     8092      +36     
  Partials       2206     2206
```


| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...o/common/extension/AdaptiveClassCodeGenerator.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vZXh0ZW5zaW9uL0FkYXB0aXZlQ2xhc3NDb2RlR2VuZXJhdG9yLmphdmE=) | `98.09% <100%> (+3.22%)` | `0 <0> (ø)` | :arrow_down: |
| [...che/dubbo/remoting/transport/mina/MinaChannel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbWluYS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcmVtb3RpbmcvdHJhbnNwb3J0L21pbmEvTWluYUNoYW5uZWwuamF2YQ==) | `43.42% <0%> (-10.53%)` | `0% <0%> (ø)` | |
| [.../remoting/transport/netty4/NettyServerHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5U2VydmVySGFuZGxlci5qYXZh) | `61.9% <0%> (-9.53%)` | `0% <0%> (ø)` | |
| [.../apache/dubbo/remoting/transport/AbstractPeer.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvQWJzdHJhY3RQZWVyLmphdmE=) | `58.69% <0%> (-8.7%)` | `0% <0%> (ø)` | |
| [...apache/dubbo/common/config/ConfigurationUtils.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vY29uZmlnL0NvbmZpZ3VyYXRpb25VdGlscy5qYXZh) | `40% <0%> (-8%)` | `0% <0%> (ø)` | |
| [...ava/org/apache/dubbo/config/DubboShutdownHook.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9EdWJib1NodXRkb3duSG9vay5qYXZh) | `75% <0%> (-6.25%)` | `0% <0%> (ø)` | |
| [...ng/transport/dispatcher/all/AllChannelHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvZGlzcGF0Y2hlci9hbGwvQWxsQ2hhbm5lbEhhbmRsZXIuamF2YQ==) | `51.42% <0%> (-5.72%)` | `0% <0%> (ø)` | |
| [.../org/apache/dubbo/remoting/ExecutionException.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9FeGVjdXRpb25FeGNlcHRpb24uamF2YQ==) | `15.78% <0%> (-5.27%)` | `0% <0%> (ø)` | |
| [.../apache/dubbo/qos/protocol/QosProtocolWrapper.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tcGx1Z2luL2R1YmJvLXFvcy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcW9zL3Byb3RvY29sL1Fvc1Byb3RvY29sV3JhcHBlci5qYXZh) | `64.1% <0%> (-5.13%)` | `0% <0%> (ø)` | |
| [...onfig/spring/extension/SpringExtensionFactory.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvZXh0ZW5zaW9uL1NwcmluZ0V4dGVuc2lvbkZhY3RvcnkuamF2YQ==) | `80.48% <0%> (-4.88%)` | `0% <0%> (ø)` | |
| ... and [6 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3489/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=footer). Last update [d33784d...761b1c2](https://codecov.io/gh/apache/incubator-dubbo/pull/3489?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3489 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] wanghbxxxx commented on issue #3489: refactor generateExtNameAssignment to reduce complexity

Posted by "wanghbxxxx (GitHub)" <gi...@apache.org>.

> I believe UT are verified, if not or UT not present could you plz add those. Thanks for simplifying the implementation 👍

actually there is no UT at all, it is covered by UT of ExtensionLoader.
i'd like to add some UT which is not easy, because there is only one public method which does a lot.
maybe i could make some of private methods public/protected which can be mocked.
could you give me some advice please!
thank you

[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3489 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org