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

Removing legacy commons-lang dependency #13480

Merged
merged 2 commits into from
Jun 27, 2024

Conversation

abhioncbr
Copy link
Contributor

Labels

  • dependencies
  • cleanup

Description

  • Removing legacy commons-lang dependency

@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2024

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes missing coverage. Please review.

Project coverage is 62.05%. Comparing base (59551e4) to head (1193e7a).
Report is 679 commits behind head on master.

Files Patch % Lines
...roller/recommender/data/DataGenerationHelpers.java 0.00% 3 Missing ⚠️
...ller/recommender/data/generator/DataGenerator.java 0.00% 3 Missing ⚠️
.../recommender/data/generator/DataGeneratorSpec.java 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #13480      +/-   ##
============================================
+ Coverage     61.75%   62.05%   +0.30%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2559     +123     
  Lines        133233   141302    +8069     
  Branches      20636    21918    +1282     
============================================
+ Hits          82274    87688    +5414     
- Misses        44911    46959    +2048     
- Partials       6048     6655     +607     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration <0.01% <0.00%> (-0.01%) ⬇️
integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration2 0.00% <0.00%> (ø)
java-11 61.99% <22.22%> (+0.28%) ⬆️
java-21 61.93% <22.22%> (+0.31%) ⬆️
skip-bytebuffers-false 62.04% <22.22%> (+0.29%) ⬆️
skip-bytebuffers-true 61.88% <22.22%> (+34.15%) ⬆️
temurin 62.05% <22.22%> (+0.30%) ⬆️
unittests 62.05% <22.22%> (+0.30%) ⬆️
unittests1 46.66% <ø> (-0.23%) ⬇️
unittests2 27.58% <22.22%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@abhioncbr abhioncbr added cleanup dependencies Pull requests that update a dependency file labels Jun 26, 2024
@abhioncbr abhioncbr marked this pull request as ready for review June 26, 2024 11:31
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up!

@@ -181,7 +181,7 @@ public File createAvroFile()
for (int i = 0; i < getCountStarResult(); i++) {
// create avro record
GenericData.Record record = new GenericData.Record(avroSchema);
record.put(ID, RandomUtils.nextInt(10));
record.put(ID, RandomUtils.nextInt(0, 10));
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems RandomUtils is already deprecated. Can you replace it with a regular Random?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did notice the deprecation warning. I plan to use commons-rng as it's much more performant than java.util.Random in the follow-up PR. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

@Jackie-Jiang Jackie-Jiang merged commit 55f6a6f into apache:master Jun 27, 2024
22 checks passed
suyashpatel98 pushed a commit to suyashpatel98/pinot that referenced this pull request Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants