Skip to content
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

fix #8378 parameter convert error #8379

merged 1 commit into from Aug 15, 2021

fix #8378 parameter convert error #8379

merged 1 commit into from Aug 15, 2021


Copy link

@24kpure 24kpure commented Jul 30, 2021

What is the purpose of the change


Brief changelog

Sevice and reference convert parameter in the same method,which is compatible with symbol(:) colon and and equal(=).As As for other config, like mock ,it works as well.

Verifying this change


  • 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.
  • 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.
  • Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
  • 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 sample in dubbo samples project.
  • GitHub Actions works fine on your own branch.
  • If this contribution is large, please follow the Software Donation Guide.

@24kpure 24kpure force-pushed the i8378 branch 2 times, most recently from 4d689fc to 5caae55 Compare Jul 30, 2021
Copy link

@codecov-commenter codecov-commenter commented Jul 30, 2021

Codecov Report

Merging #8379 (51219ce) into master (ca794b6) will decrease coverage by 0.12%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #8379      +/-   ##
- Coverage     61.20%   61.07%   -0.13%     
- Complexity      446      448       +2     
  Files          1096     1097       +1     
  Lines         44223    44401     +178     
  Branches       6445     6469      +24     
+ Hits          27066    27119      +53     
- Misses        14176    14296     +120     
- Partials       2981     2986       +5     
Impacted Files Coverage Δ
...beans/factory/annotation/ 94.82% <66.66%> (+1.27%) ⬆️
...dubbo/config/spring/util/ 50.00% <85.00%> (+17.50%) ⬆️
.../factory/annotation/ 87.75% <100.00%> (+0.65%) ⬆️ 52.45% <0.00%> (-18.04%) ⬇️ 82.75% <0.00%> (-6.90%) ⬇️
...apache/dubbo/rpc/protocol/injvm/ 74.35% <0.00%> (-5.13%) ⬇️
...port/identifier/ 57.14% <0.00%> (-3.58%) ⬇️
...he/dubbo/remoting/transport/netty/ 70.17% <0.00%> (-3.51%) ⬇️
...o/rpc/cluster/support/migration/ 42.10% <0.00%> (-2.64%) ⬇️
...ting/zookeeper/curator/ 68.96% <0.00%> (-1.15%) ⬇️
... and 17 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 ca794b6...51219ce. Read the comment docs.

Copy link

@kylixs kylixs commented Aug 2, 2021

@24kpure please add some unit tests for parameters converting method and list all supported formats in java doc.

Copy link
Contributor Author

@24kpure 24kpure commented Aug 2, 2021

@24kpure please add some unit tests for parameters converting method and list all supported formats in java doc.

Thanks for your advice.I have added a unit test for paramters converting method and listed supported formats in java doc.It‘s my first time to add unit test in Dubbo.Why not give more advice to me?
Many thanks.

@AlbumenJ AlbumenJ requested a review from kylixs Aug 3, 2021
Copy link
Contributor Author

@24kpure 24kpure commented Aug 12, 2021

@kylixs Is there something wrong with the pr?

@AlbumenJ AlbumenJ added this to the 2.7.14 milestone Aug 15, 2021
@AlbumenJ AlbumenJ merged commit 6acd5a2 into apache:master Aug 15, 2021
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

4 participants