You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "luchy0120 (GitHub)" <gi...@apache.org> on 2018/12/11 02:21:50 UTC

[GitHub] [incubator-dubbo] luchy0120 opened pull request #2936: standardize semantics of all mergers,enhance mergeFactory and testcases

## What is the purpose of the change

This patch can standardize semantics of all mergers :  merge arguments can be null , and all the others should have same type . 

For ArrayMerger , as its arguments type is  Object[]... items ,  so we can avoid checking whether item[i] is
an Array . Also , in order to avoid problems like 
```
        class Person{};
        class Teacher extends Person{};
        Person[] a= {new Person()};
        Teacher[] b= {new Teacher()};
        Object[] result = ArrayMerger.INSTANCE.merge(a,b);
```
we can require all elements to have the same type , al least the user should pass in the same type

For MergerFactory , if the required class is not in cache , we can throw an user friendly Exception 

For test case , I add some exception case , and fix an ArrayMerger case , also add some null argument
test case . 


## Brief changelog

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/merger/*
dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/merger/ResultMergerTest


## Verifying this change

mvn clean install -DskipTests


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

[GitHub] [incubator-dubbo] beiwei30 closed pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
[ pull request closed by beiwei30 ]

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

[GitHub] [incubator-dubbo] beiwei30 commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
@luchy0120 could you pls. take a look?

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

[GitHub] [incubator-dubbo] beiwei30 commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
I think it's ready to go.

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

[GitHub] [incubator-dubbo] luchy0120 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "luchy0120 (GitHub)" <gi...@apache.org>.
if I use "if != null ",  there may be too much {{}} ,  会形成嵌套的if语句

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

[GitHub] [incubator-dubbo] luchy0120 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

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

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

[GitHub] [incubator-dubbo] codecov-io commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=h1) Report
> Merging [#2936](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/795a84005fb86a6fc3649810807624e39e3bdd5c?src=pr&el=desc) will **increase** coverage by `0.1%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff            @@
##           master    #2936     +/-   ##
=========================================
+ Coverage   63.73%   63.84%   +0.1%     
=========================================
  Files         578      578             
  Lines       25959    26007     +48     
  Branches     4544     4566     +22     
=========================================
+ Hits        16546    16605     +59     
+ Misses       7241     7234      -7     
+ Partials     2172     2168      -4
```


| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rg/apache/dubbo/rpc/cluster/merger/ListMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xpc3RNZXJnZXIuamF2YQ==) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/ShortArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1Nob3J0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...g/apache/dubbo/rpc/cluster/merger/ArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0FycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (+31.57%)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/LongArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xvbmdBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/ByteArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0J5dGVBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...org/apache/dubbo/rpc/cluster/merger/SetMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1NldE1lcmdlci5qYXZh) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...he/dubbo/rpc/cluster/merger/DoubleArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0RvdWJsZUFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/FloatArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Zsb2F0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...pache/dubbo/rpc/cluster/merger/IntArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0ludEFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...e/dubbo/rpc/cluster/merger/BooleanArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Jvb2xlYW5BcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| ... and [10 more](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936?src=pr&el=footer). Last update [795a840...de12c48](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] luchy0120 commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "luchy0120 (GitHub)" <gi...@apache.org>.
@beiwei30, @kimmking,  I've updated code accroding to your review . I add some comments for mergeFactory and remove the unnecessary check 

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

[GitHub] [incubator-dubbo] kimmking commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "kimmking (GitHub)" <gi...@apache.org>.
why check this?

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

[GitHub] [incubator-dubbo] beiwei30 commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
nice suggestion, I will take a look.

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

[GitHub] [incubator-dubbo] codecov-io commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=h1) Report
> Merging [#2936](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/9d4b6cbaae3adce765b0b1dba6a50a92e504c7b1?src=pr&el=desc) will **increase** coverage by `0.04%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master    #2936      +/-   ##
==========================================
+ Coverage   63.92%   63.97%   +0.04%     
==========================================
  Files         584      584              
  Lines       26005    26051      +46     
  Branches     4543     4564      +21     
==========================================
+ Hits        16624    16665      +41     
- Misses       7195     7204       +9     
+ Partials     2186     2182       -4
```


| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rg/apache/dubbo/rpc/cluster/merger/ListMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xpc3RNZXJnZXIuamF2YQ==) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/ShortArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1Nob3J0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...g/apache/dubbo/rpc/cluster/merger/ArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0FycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (+31.57%)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/LongArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xvbmdBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/ByteArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0J5dGVBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...org/apache/dubbo/rpc/cluster/merger/SetMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1NldE1lcmdlci5qYXZh) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...he/dubbo/rpc/cluster/merger/DoubleArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0RvdWJsZUFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/FloatArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Zsb2F0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...pache/dubbo/rpc/cluster/merger/IntArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0ludEFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...e/dubbo/rpc/cluster/merger/BooleanArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Jvb2xlYW5BcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| ... and [19 more](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936?src=pr&el=footer). Last update [9d4b6cb...91f064a](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] codecov-io commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=h1) Report
> Merging [#2936](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/9d4b6cbaae3adce765b0b1dba6a50a92e504c7b1?src=pr&el=desc) will **increase** coverage by `0.06%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master    #2936      +/-   ##
==========================================
+ Coverage   63.92%   63.98%   +0.06%     
==========================================
  Files         584      584              
  Lines       26005    26051      +46     
  Branches     4543     4564      +21     
==========================================
+ Hits        16624    16670      +46     
- Misses       7195     7205      +10     
+ Partials     2186     2176      -10
```


| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rg/apache/dubbo/rpc/cluster/merger/ListMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xpc3RNZXJnZXIuamF2YQ==) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/ShortArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1Nob3J0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...g/apache/dubbo/rpc/cluster/merger/ArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0FycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (+31.57%)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/LongArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xvbmdBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/ByteArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0J5dGVBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...org/apache/dubbo/rpc/cluster/merger/SetMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1NldE1lcmdlci5qYXZh) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...he/dubbo/rpc/cluster/merger/DoubleArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0RvdWJsZUFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/FloatArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Zsb2F0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...pache/dubbo/rpc/cluster/merger/IntArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0ludEFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...e/dubbo/rpc/cluster/merger/BooleanArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Jvb2xlYW5BcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| ... and [17 more](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936?src=pr&el=footer). Last update [9d4b6cb...78215d7](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] beiwei30 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
Pls. use Collections.emptyList() instead.

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

[GitHub] [incubator-dubbo] beiwei30 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
pls. use Collections.emptyMap() instead.

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

[GitHub] [incubator-dubbo] beiwei30 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
pls. use Collections.emptySet()

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

[GitHub] [incubator-dubbo] codecov-io commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=h1) Report
> Merging [#2936](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/9d4b6cbaae3adce765b0b1dba6a50a92e504c7b1?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master    #2936      +/-   ##
==========================================
+ Coverage   63.92%   63.96%   +0.03%     
==========================================
  Files         584      584              
  Lines       26005    26051      +46     
  Branches     4543     4564      +21     
==========================================
+ Hits        16624    16663      +39     
- Misses       7195     7213      +18     
+ Partials     2186     2175      -11
```


| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rg/apache/dubbo/rpc/cluster/merger/ListMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xpc3RNZXJnZXIuamF2YQ==) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/ShortArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1Nob3J0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...g/apache/dubbo/rpc/cluster/merger/ArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0FycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (+31.57%)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/LongArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xvbmdBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/ByteArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0J5dGVBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...org/apache/dubbo/rpc/cluster/merger/SetMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1NldE1lcmdlci5qYXZh) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...he/dubbo/rpc/cluster/merger/DoubleArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0RvdWJsZUFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/FloatArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Zsb2F0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...pache/dubbo/rpc/cluster/merger/IntArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0ludEFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...e/dubbo/rpc/cluster/merger/BooleanArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Jvb2xlYW5BcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| ... and [15 more](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936?src=pr&el=footer). Last update [9d4b6cb...9761fbc](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] beiwei30 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
I don't think we should throw an exception, let's just return null.

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

[GitHub] [incubator-dubbo] luchy0120 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

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

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

[GitHub] [incubator-dubbo] luchy0120 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

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

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

[GitHub] [incubator-dubbo] luchy0120 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "luchy0120 (GitHub)" <gi...@apache.org>.
This is not wrong . It means "result[index++] = array[j]"

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

[GitHub] [incubator-dubbo] kimmking commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "kimmking (GitHub)" <gi...@apache.org>.
use if !=null then ...

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

[GitHub] [incubator-dubbo] luchy0120 commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "luchy0120 (GitHub)" <gi...@apache.org>.
#2926 

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

[GitHub] [incubator-dubbo] codecov-io commented on issue #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=h1) Report
> Merging [#2936](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/e508ad467f94e853885fdf47a74d312eb5c6001b?src=pr&el=desc) will **increase** coverage by `0.07%`.
> The diff coverage is `98.86%`.

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

```diff
@@            Coverage Diff             @@
##           master    #2936      +/-   ##
==========================================
+ Coverage   63.82%   63.89%   +0.07%     
==========================================
  Files         578      578              
  Lines       25954    26002      +48     
  Branches     4543     4565      +22     
==========================================
+ Hits        16565    16615      +50     
- Misses       7220     7223       +3     
+ Partials     2169     2164       -5
```


| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rg/apache/dubbo/rpc/cluster/merger/ListMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xpc3RNZXJnZXIuamF2YQ==) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/ShortArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1Nob3J0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/LongArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0xvbmdBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/ByteArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0J5dGVBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...org/apache/dubbo/rpc/cluster/merger/SetMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL1NldE1lcmdlci5qYXZh) | `100% <100%> (+16.66%)` | :arrow_up: |
| [...he/dubbo/rpc/cluster/merger/DoubleArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0RvdWJsZUFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...che/dubbo/rpc/cluster/merger/FloatArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Zsb2F0QXJyYXlNZXJnZXIuamF2YQ==) | `100% <100%> (ø)` | :arrow_up: |
| [...pache/dubbo/rpc/cluster/merger/IntArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0ludEFycmF5TWVyZ2VyLmphdmE=) | `100% <100%> (ø)` | :arrow_up: |
| [...e/dubbo/rpc/cluster/merger/BooleanArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0Jvb2xlYW5BcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| [...ache/dubbo/rpc/cluster/merger/CharArrayMerger.java](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree#diff-ZHViYm8tY2x1c3Rlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcnBjL2NsdXN0ZXIvbWVyZ2VyL0NoYXJBcnJheU1lcmdlci5qYXZh) | `100% <100%> (ø)` | :arrow_up: |
| ... and [16 more](https://codecov.io/gh/apache/incubator-dubbo/pull/2936/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936?src=pr&el=footer). Last update [e508ad4...3612123](https://codecov.io/gh/apache/incubator-dubbo/pull/2936?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/2936 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org

[GitHub] [incubator-dubbo] luchy0120 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "luchy0120 (GitHub)" <gi...@apache.org>.
May be we can add some comments about this NullPointerException

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

[GitHub] [incubator-dubbo] beiwei30 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
I still think it is ok to return null.

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

[GitHub] [incubator-dubbo] kimmking commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "kimmking (GitHub)" <gi...@apache.org>.
array[index++] =result?

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

[GitHub] [incubator-dubbo] luchy0120 commented on pull request #2936: [Dubbo 2926] standardize semantics of all mergers,enhance mergeFactory and testcases

Posted by "luchy0120 (GitHub)" <gi...@apache.org>.
Because if the user pass in (   null ,null , new String[]{"abs","edf"} , null   ) ,  we need to find out the  String.class  , so we need to skip the null objects . 

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