Skip to content

replace CollectionUtils deprecated method#3027

Merged
qiaozhanwei merged 23 commits intoapache:devfrom
CalvinKirs:benMap
Jul 13, 2020
Merged

replace CollectionUtils deprecated method#3027
qiaozhanwei merged 23 commits intoapache:devfrom
CalvinKirs:benMap

Conversation

@CalvinKirs
Copy link
Copy Markdown
Member

Tips

What is the purpose of the pull request

replace CollectionUtils deprecated method

Verify this pull request

(Please pick either of the following options)

This pull request is already covered by existing tests, such as (please describe tests).

@CalvinKirs
Copy link
Copy Markdown
Member Author

CalvinKirs commented Jun 21, 2020

BeanUtils Check dependency license fail,so,I copied it to my project

@gabry-lab
Copy link
Copy Markdown
Member

@CalvinKirs SonarCloud Code Analysis test failed

@CalvinKirs
Copy link
Copy Markdown
Member Author

@CalvinKirs SonarCloud Code Analysis test failed

commons-bean Check dependency license fail ,so,I copied bean-map to my project,I don’t know if it’s acceptable or not, I hope to get some suggestions from you

@gabry-lab
Copy link
Copy Markdown
Member

@CalvinKirs SonarCloud Code Analysis test failed

commons-bean Check dependency license fail ,so,I copied bean-map to my project,I don’t know if it’s acceptable or not, I hope to get some suggestions from you

@CalvinKirs If the beanMap class deprecated, so there is an alternative ? Only copying it might not be a good solution

@CalvinKirs
Copy link
Copy Markdown
Member Author

@CalvinKirs SonarCloud Code Analysis test failed

commons-bean Check dependency license fail ,so,I copied bean-map to my project,I don’t know if it’s acceptable or not, I hope to get some suggestions from you

@CalvinKirs If the beanMap class deprecated, so there is an alternative ? Only copying it might not be a good solution

Thank you very much for your suggestion. You are right. In fact, I also feel that it is rude to directly copy this method, I think I can achieve the same function in other ways. I will finish it later it.

@CalvinKirs
Copy link
Copy Markdown
Member Author

I have completed the change, I’m sorry to trouble you to have time to review @gabrywu

@lgcareer
Copy link
Copy Markdown
Contributor

lgcareer commented Jun 22, 2020

Hi,you should describe detail as following when update the pom dependency.
1、Which pom dependency you removed?And which jar removed?
2、 Which pom dependency you added?And which jar added?
Except for the above,You should give the github usrl of the dependency version.

@CalvinKirs
Copy link
Copy Markdown
Member Author

Hi,you should describe detail as following when update the pom dependency.
1、Which pom dependency you removed?And which jar removed?
2、 Which pom dependency you added?And which jar added?
Except for the above,You should give the github usrl of the dependency version.

Thank you very much for your review. I didn’t remove the jar. I changed it to beanMap because the original method was deprecated. I added common-beanutils, which was originally in the parent pom, but no license was added, I fixed it It, its address is https://github.com/apache/commons-beanutils

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #3027 into dev will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #3027      +/-   ##
============================================
- Coverage     33.39%   33.38%   -0.02%     
+ Complexity     2373     2371       -2     
============================================
  Files           441      441              
  Lines         20518    20518              
  Branches       2505     2505              
============================================
- Hits           6853     6850       -3     
- Misses        13018    13020       +2     
- Partials        647      648       +1     
Impacted Files Coverage Δ Complexity Δ
...dolphinscheduler/common/utils/CollectionUtils.java 92.53% <ø> (ø) 30.00 <0.00> (ø)
...he/dolphinscheduler/api/service/LoggerService.java 82.60% <0.00%> (-8.70%) 9.00% <0.00%> (-1.00%)
...dolphinscheduler/remote/future/ResponseFuture.java 55.93% <0.00%> (-1.70%) 10.00% <0.00%> (-1.00%)

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 2d0fe8a...03cd407. Read the comment docs.

Copy link
Copy Markdown
Contributor

@qiaozhanwei qiaozhanwei left a comment

Choose a reason for hiding this comment

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

+1

@qiaozhanwei
Copy link
Copy Markdown
Contributor

please resolve conflicting files . Thx

Copy link
Copy Markdown
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

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

+1
good job

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@CalvinKirs
Copy link
Copy Markdown
Member Author

please resolve conflicting files . Thx

Thanks for your review, I have completed the changes, please check again, Thx

@qiaozhanwei qiaozhanwei merged commit 38e7494 into apache:dev Jul 13, 2020
@davidzollo davidzollo added the enhancement New feature or request label Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants