Skip to content

Remove impossible reach branch and the useless variable.#6862

Merged
htynkn merged 7 commits intoapache:masterfrom
Chiyuxing:master
Nov 9, 2020
Merged

Remove impossible reach branch and the useless variable.#6862
htynkn merged 7 commits intoapache:masterfrom
Chiyuxing:master

Conversation

@Chiyuxing
Copy link
Copy Markdown

@Chiyuxing Chiyuxing commented Nov 2, 2020 •
edited
Loading

What is the purpose of the change

  1. Clean code.
  2. Improve performence.

Brief changelog

Remove the useless variable success and the associated unreachable branche.

Verifying this change

Build and Tested

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 sample in dubbo samples project.
  • 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.

@leechor
Copy link
Copy Markdown
Contributor

leechor commented Nov 3, 2020

LGTM

@htynkn
Copy link
Copy Markdown
Member

htynkn commented Nov 6, 2020

I believe doUnregister method and doSubscribe method may has similar issue. Could you please help to check and include them in this PR?

@htynkn htynkn added the status/waiting-for-feedback Need reporters to triage label Nov 6, 2020
@Chiyuxing
Copy link
Copy Markdown
Author

Chiyuxing commented Nov 6, 2020

I believe doUnregister method and doSubscribe method may has similar issue. Could you please help to check and include them in this PR?

With pleasure.

@htynkn htynkn self-assigned this Nov 6, 2020
@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 6, 2020 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@27917f2). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6862   +/-   ##
=========================================
  Coverage          ?   59.07%           
  Complexity        ?      508           
=========================================
  Files             ?     1028           
  Lines             ?    41521           
  Branches          ?     6035           
=========================================
  Hits              ?    24529           
  Misses            ?    14219           
  Partials          ?     2773

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 27917f2...3bff5dc. Read the comment docs.

@Chiyuxing
Copy link
Copy Markdown
Author

Chiyuxing commented Nov 6, 2020

@htynkn I have done. Pls review it again.

@htynkn htynkn added status/READY-TO-MERGE and removed status/waiting-for-feedback Need reporters to triage labels Nov 9, 2020
@htynkn htynkn merged commit 1a3949e into apache:master Nov 9, 2020
@htynkn
Copy link
Copy Markdown
Member

htynkn commented Nov 9, 2020

I did some small change to revert some unrelated code format.
Your PR has been merge into master branch now. Thanks for your contribution

@Chiyuxing
Copy link
Copy Markdown
Author

Chiyuxing commented Nov 9, 2020

I did some small change to revert some unrelated code format.
Your PR has been merge into master branch now. Thanks for your contribution

Great. Thanks for your assistance.

chickenlj pushed a commit to chickenlj/incubator-dubbo that referenced this pull request Nov 25, 2020
@Chiyuxing @chickenlj
remove impossible reach branch and the useless variable for RedisRegi…
b9e577e 
…stry.java (apache#6862)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@htynkn htynkn htynkn approved these changes

Assignees

@htynkn htynkn

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Chiyuxing @leechor @htynkn @codecov-io