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

Merge exceptions, remove redundant interface implementations #4730

Merged
merged 10 commits into from Apr 21, 2021

Conversation

CrazyHZM
Copy link
Member

@CrazyHZM CrazyHZM commented Aug 3, 2019

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:

  • 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.

@CrazyHZM CrazyHZM added this to In progress in make code clean via automation Aug 3, 2019
Copy link

@Moriadry-zz Moriadry-zz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Nice clean up:)

final TServer thriftServer = new TThreadedSelectorServer(tArgs);
serverMap.put(url.getAddress(),thriftServer);
final TServer thriftServer = new TThreadedSelectorServer(tArgs);
serverMap.put(url.getAddress(), thriftServer);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a method(such as a pattern?) to make all those formatter done? I found many this kind of problems.
Like

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different protocols have different implementations, and I don't think there is a need to use design patterns here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean the lack of space here, not design pattern, but code format.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, understand the mistake, modify it later.

@codecov-io
Copy link

codecov-io commented Aug 6, 2019

Codecov Report

Merging #4730 (63208f2) into master (b7bd6ad) will decrease coverage by 0.10%.
The diff coverage is 67.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #4730      +/-   ##
============================================
- Coverage     59.22%   59.11%   -0.11%     
- Complexity      503      527      +24     
============================================
  Files          1076     1076              
  Lines         43410    43410              
  Branches       6339     6340       +1     
============================================
- Hits          25709    25662      -47     
- Misses        14858    14906      +48     
+ Partials       2843     2842       -1     
Impacted Files Coverage Δ Complexity Δ
...apache/dubbo/rpc/protocol/injvm/InjvmProtocol.java 77.77% <ø> (ø) 0.00 <0.00> (ø)
...org/apache/dubbo/rpc/protocol/rmi/RmiProtocol.java 0.00% <ø> (-61.67%) 0.00 <0.00> (-10.00)
...ubbo/rpc/protocol/nativethrift/ThriftProtocol.java 69.87% <67.50%> (+1.58%) 8.00 <2.00> (+1.00)
...he/dubbo/rpc/protocol/rmi/RmiRemoteInvocation.java 0.00% <0.00%> (-80.00%) 0.00% <0.00%> (-2.00%)
...che/dubbo/remoting/transport/mina/MinaChannel.java 35.52% <0.00%> (-10.53%) 14.00% <0.00%> (-1.00%)
...src/main/java/org/apache/dubbo/common/Version.java 68.93% <0.00%> (-4.86%) 0.00% <0.00%> (ø%)
.../dubbo/remoting/transport/netty4/NettyChannel.java 61.38% <0.00%> (-2.98%) 0.00% <0.00%> (ø%)
...src/main/java/org/apache/dubbo/rpc/RpcContext.java 66.33% <0.00%> (-0.50%) 0.00% <0.00%> (ø%)
.../rpc/cluster/configurator/parser/ConfigParser.java 85.84% <0.00%> (-0.13%) 0.00% <0.00%> (ø%)
... and 6 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 b7bd6ad...63208f2. Read the comment docs.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

# Conflicts:
#	dubbo-rpc/dubbo-rpc-native-thrift/src/main/java/org/apache/dubbo/rpc/protocol/nativethrift/ThriftProtocol.java
@AlbumenJ
Copy link
Member

@CrazyHZM Hi, pls resolve conficts.

@AlbumenJ AlbumenJ added the status/waiting-for-feedback Need reporters to triage label Apr 11, 2021
@CrazyHZM
Copy link
Member Author

@AlbumenJ done

@AlbumenJ AlbumenJ merged commit 0d00fe0 into apache:master Apr 21, 2021
@CrazyHZM CrazyHZM deleted the cleanCode branch April 27, 2021 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/waiting-for-feedback Need reporters to triage
Projects
No open projects
make code clean
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

5 participants