You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "biyuhao (GitHub)" <gi...@apache.org> on 2018/12/31 11:11:20 UTC
[GitHub] [incubator-dubbo-ops] biyuhao opened pull request #214:
Minor fix
Fix string comparison
Fix possible index out of bound
[ Full content available at: https://github.com/apache/incubator-dubbo-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] biyuhao commented on pull request
#214: Minor fix
Posted by "biyuhao (GitHub)" <gi...@apache.org>.
>From the exception info below, I think the logic here is meant to check both `registrlUrl` and `registryAddress ` are empty.
[ Full content available at: https://github.com/apache/incubator-dubbo-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] nzomkxia closed pull request #214:
Minor fix
Posted by "nzomkxia (GitHub)" <gi...@apache.org>.
[ pull request closed by nzomkxia ]
[ Full content available at: https://github.com/apache/incubator-dubbo-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] codecov-io commented on issue #214:
Minor fix
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?src=pr&el=h1) Report
> Merging [#214](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?src=pr&el=desc) into [develop](https://codecov.io/gh/apache/incubator-dubbo-ops/commit/23049f5db205d3ac9b0685e2a784c230ea507d07?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #214 +/- ##
============================================
+ Coverage 0.57% 0.57% +<.01%
Complexity 7 7
============================================
Files 82 82
Lines 3284 3280 -4
Branches 521 521
============================================
Hits 19 19
+ Misses 3264 3260 -4
Partials 1 1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...va/org/apache/dubbo/admin/config/ConfigCenter.java](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214/diff?src=pr&el=tree#diff-ZHViYm8tYWRtaW4tYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vYWRtaW4vY29uZmlnL0NvbmZpZ0NlbnRlci5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...che/dubbo/admin/service/impl/RouteServiceImpl.java](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214/diff?src=pr&el=tree#diff-ZHViYm8tYWRtaW4tYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vYWRtaW4vc2VydmljZS9pbXBsL1JvdXRlU2VydmljZUltcGwuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?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-ops/pull/214?src=pr&el=footer). Last update [23049f5...467b9d5](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?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-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] nzomkxia commented on issue #214:
Minor fix
Posted by "nzomkxia (GitHub)" <gi...@apache.org>.
Thank you for your pr, I'll merge it
[ Full content available at: https://github.com/apache/incubator-dubbo-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] biyuhao commented on pull request
#214: Minor fix
Posted by "biyuhao (GitHub)" <gi...@apache.org>.
I did a little test with condition string is `"=>"`
After `condition.split("=>", 2);` and `array[0].trim();` executed
all possible whitespace is trimed, so I personally think is ok to check with `length() != 0`
[ Full content available at: https://github.com/apache/incubator-dubbo-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] codecov-io commented on issue #214:
Minor fix
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?src=pr&el=h1) Report
> Merging [#214](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?src=pr&el=desc) into [develop](https://codecov.io/gh/apache/incubator-dubbo-ops/commit/23049f5db205d3ac9b0685e2a784c230ea507d07?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #214 +/- ##
============================================
+ Coverage 0.57% 0.57% +<.01%
Complexity 7 7
============================================
Files 82 82
Lines 3284 3280 -4
Branches 521 521
============================================
Hits 19 19
+ Misses 3264 3260 -4
Partials 1 1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...va/org/apache/dubbo/admin/config/ConfigCenter.java](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214/diff?src=pr&el=tree#diff-ZHViYm8tYWRtaW4tYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vYWRtaW4vY29uZmlnL0NvbmZpZ0NlbnRlci5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...che/dubbo/admin/service/impl/RouteServiceImpl.java](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214/diff?src=pr&el=tree#diff-ZHViYm8tYWRtaW4tYmFja2VuZC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vYWRtaW4vc2VydmljZS9pbXBsL1JvdXRlU2VydmljZUltcGwuamF2YQ==) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?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-ops/pull/214?src=pr&el=footer). Last update [23049f5...467b9d5](https://codecov.io/gh/apache/incubator-dubbo-ops/pull/214?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-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] lovepoem commented on pull request
#214: Minor fix
Posted by "lovepoem (GitHub)" <gi...@apache.org>.
I think it should be `StringUtils.isNotBlank `
[ Full content available at: https://github.com/apache/incubator-dubbo-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] biyuhao commented on pull request
#214: Minor fix
Posted by "biyuhao (GitHub)" <gi...@apache.org>.
>From the exception info blow, I think the logic here is meant to check both `registrlUrl` and `registryAddress ` are empty.
[ Full content available at: https://github.com/apache/incubator-dubbo-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] biyuhao commented on pull request
#214: Minor fix
Posted by "biyuhao (GitHub)" <gi...@apache.org>.
We are building a `org.apache.dubbo.common.URL` object,
if we use `URL#valueOf()` when input string is null or malformed we got `throw new IllegalArgumentException("url == null");`.
Shall we leave the Inspection to `valueOf` internal here?
[ Full content available at: https://github.com/apache/incubator-dubbo-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo-ops] lovepoem commented on pull request
#214: Minor fix
Posted by "lovepoem (GitHub)" <gi...@apache.org>.
Change the function ? I think it should be `StringUtils.isNotBlank `
[ Full content available at: https://github.com/apache/incubator-dubbo-ops/pull/214 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org