Skip to content

[Improvement] Fix unchecked type conversions#6003

Merged
ruanwenjun merged 2 commits intoapache:devfrom
lyxell:fix-unchecked-type-conversions
Aug 20, 2021
Merged

[Improvement] Fix unchecked type conversions#6003
ruanwenjun merged 2 commits intoapache:devfrom
lyxell:fix-unchecked-type-conversions

Conversation

@lyxell
Copy link

@lyxell lyxell commented Aug 18, 2021

Fix compiler warnings about unchecked type conversions.

Brief change log

  • Fix compiler warnings about unchecked type conversions in DAG.java, ResourceProcessDefinitionUtilsTest.java, NettyExecutorManager.java and MasterExecThreadTest.java

Verify this pull request

No functional changes, test coverage is not affected.


This patch was generated automatically using Logifix.

Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

LGTM

And I have a question, when should we use Collections.emptySet in program, becase this method return an immutable set, If the upper layer want to add elements, they need to create a new HashSet.

@lyxell
Copy link
Author

lyxell commented Aug 19, 2021

LGTM

Great!

And I have a question, when should we use Collections.emptySet in program, becase this method return an immutable set, If the upper layer want to add elements, they need to create a new HashSet.

I think Collections.emptySet is a good default. If the user needs a modifiable set they can always make a copy.

@ruanwenjun
Copy link
Member

ruanwenjun commented Aug 19, 2021

@lyxell OK, I just feel it's troublesome to do a copy whenever get a set or list. Maybe it's common in functional scenarios.

And you need to fix the code style.

@lyxell
Copy link
Author

lyxell commented Aug 19, 2021

@ruanwenjun Sure, this pull request did not introduce that checkstyle error, but I can fix it anyway.

@codecov-commenter
Copy link

codecov-commenter commented Aug 19, 2021

Codecov Report

Merging #6003 (db6db88) into dev (4b892f7) will decrease coverage by 0.00%.
The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #6003      +/-   ##
============================================
- Coverage     46.75%   46.75%   -0.01%     
+ Complexity     3744     3743       -1     
============================================
  Files           604      604              
  Lines         24829    24829              
  Branches       2836     2836              
============================================
- Hits          11610    11608       -2     
- Misses        12077    12078       +1     
- Partials       1142     1143       +1     
Impacted Files Coverage Δ
...master/dispatch/executor/NettyExecutorManager.java 0.00% <0.00%> (ø)
.../org/apache/dolphinscheduler/common/graph/DAG.java 96.03% <100.00%> (ø)
...er/master/dispatch/host/assign/RandomSelector.java 77.77% <0.00%> (-5.56%) ⬇️
...heduler/server/master/runner/MasterExecThread.java 16.08% <0.00%> (-0.19%) ⬇️

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 4b892f7...db6db88. Read the comment docs.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@ruanwenjun ruanwenjun merged commit ae386dd into apache:dev Aug 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants