Skip to content

Conversation

@sullis
Copy link
Contributor

@sullis sullis commented Apr 2, 2024

No description provided.

@sullis sullis force-pushed the sean/MinionClient-TypeReference branch from 8314316 to ce6b0d1 Compare April 2, 2024 12:24
@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 35.71429% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 62.11%. Comparing base (59551e4) to head (5bc4376).
Report is 244 commits behind head on master.

Files Patch % Lines
...a/org/apache/pinot/common/minion/MinionClient.java 35.71% 9 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #12776      +/-   ##
============================================
+ Coverage     61.75%   62.11%   +0.36%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2492      +56     
  Lines        133233   135952    +2719     
  Branches      20636    21036     +400     
============================================
+ Hits          82274    84443    +2169     
- Misses        44911    45262     +351     
- Partials       6048     6247     +199     
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 62.06% <35.71%> (+0.35%) ⬆️
java-21 61.96% <35.71%> (+0.33%) ⬆️
skip-bytebuffers-false 62.10% <35.71%> (+0.35%) ⬆️
skip-bytebuffers-true 61.91% <35.71%> (+34.18%) ⬆️
temurin 62.11% <35.71%> (+0.36%) ⬆️
unittests 62.10% <35.71%> (+0.36%) ⬆️
unittests1 46.97% <35.71%> (+0.08%) ⬆️
unittests2 27.75% <0.00%> (+0.02%) ⬆️

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.

@sullis
Copy link
Contributor Author

sullis commented Apr 2, 2024

ready for review

@sullis sullis force-pushed the sean/MinionClient-TypeReference branch 2 times, most recently from e440fc5 to f6b7ad1 Compare April 4, 2024 17:33
@sullis
Copy link
Contributor Author

sullis commented Apr 4, 2024

please review @gortiz @Jackie-Jiang

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.

Let's add a method Map<String, Object> stringToMap(String jsonString) into JsonUtils

@sullis sullis changed the title use static TypeReference object introduce JsonUtils stringToMap Apr 4, 2024
@sullis
Copy link
Contributor Author

sullis commented Apr 4, 2024

Let's add a method Map<String, Object> stringToMap(String jsonString) into JsonUtils

would you prefer Map<String, Object> or Map<String, String>

MinionClient uses Map<String, String>

@Jackie-Jiang
Copy link
Contributor

Sorry didn't notice these 2 references are different.. In that case the previous commit is more clear

@sullis sullis changed the title introduce JsonUtils stringToMap use static TypeReference object Apr 5, 2024
@sullis
Copy link
Contributor Author

sullis commented Apr 5, 2024

Sorry didn't notice these 2 references are different.. In that case the previous commit is more clear

Ack. I have removed JsonUtils stringToMap. I just pushed the revert.

@sullis sullis force-pushed the sean/MinionClient-TypeReference branch 2 times, most recently from d561db1 to 9d35bb4 Compare April 5, 2024 23:56
@sullis
Copy link
Contributor Author

sullis commented Apr 6, 2024

Test suite passes

@sullis
Copy link
Contributor Author

sullis commented Apr 8, 2024

Ready for review

@sullis sullis force-pushed the sean/MinionClient-TypeReference branch from 9d35bb4 to 97dfc8f Compare April 8, 2024 22:06
@sullis sullis force-pushed the sean/MinionClient-TypeReference branch from 97dfc8f to 5bc4376 Compare April 9, 2024 02:14
@Jackie-Jiang Jackie-Jiang merged commit 7e4ea06 into apache:master Apr 9, 2024
@sullis sullis deleted the sean/MinionClient-TypeReference branch April 10, 2024 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants