[#2150] Improvement : Arguments in incorrect order fixed with correct order

[ajayl83]

What changes were proposed in this pull request?

• Arguments in incorrect order fixed with correct order!

Why are the changes needed?

• Fix: [Improvement] In DTOConverters.java arguments in incorect order? #2150

Does this PR introduce any user-facing change?

• No

How was this patch tested?

• ./gradlew test -PskipITs

[justinmclean]

Hi it looks like you have more than one issue here can you revert the changes related to Distribution?

[ajayl83]

Hi it looks like you have more than one issue here can you revert the changes related to Distribution?

oh yes , apologies i have reverted the commit

[justinmclean]

Thanks for fixing that.

[jerryshao]

Would you please add a UT to cover this change? If you're not able to write a UT, @mchades can you please assist @ajayl83 to achieve this?

[ajayl83]

Would you please add a UT to cover this change? If you're not able to write a UT, @mchades can you please assist @ajayl83 to achieve this?

@jerryshao thats not a problem , i will add the test for it

[ajayl83]

@jerryshao i dont see existing test so can i add only for this method or all :)

[mchades]

It's better to add all tests for partitionDTO(IdentityPartitionDTO, RangePartitionDTO and ListPartitionDTO) in com.datastrato.gravitino.json.TestJsonUtils

[ajayl83]

@jerryshao @mchades i added the necessary tests however while running the build against jdk17 i am coming across below error, can you kindly let me know how to fix this?
Task :rat FAILED
Files with unapproved licenses:

• /Users/ajaylele/Downloads/gravitino/common/src/test/java/com/datastrato/gravitino/dto/util/DTOConvertersTest.java

[mchades]

You can push the codes, so that we can see detailed information from GitHub's CI.

[jerryshao]

@jerryshao @mchades i added the necessary tests however while running the build against jdk17 i am coming across below error, can you kindly let me know how to fix this? Task :rat FAILED Files with unapproved licenses:

• /Users/ajaylele/Downloads/gravitino/common/src/test/java/com/datastrato/gravitino/dto/util/DTOConvertersTest.java

Seems like the license header issue, did you add the license header to this DTOConvertersTest.java file?

[ajayl83]

@jerryshao there is already license header, the other problem was i was using static import, i fixed it but still build fails with same problem :(

[justinmclean]

If you push your changes we should be able to work out what the issue is.

[ajayl83]

@jerryshao my bad i earlier wrote new UT for this, but have moved now to TestJsonUtils.java :) , its good now, will commit it

[jerryshao]

@mchades would you please help to review.

[ajayl83]

@mchades i am having weird issue with spotlessJava formatting , if i execute this all the imports are gone, can you kindly assist?

[ajayl83]

@mchades its my IDE problem, do we have a settings file that can be imported for Intellij?

[mchades]

FYI:
settings -> Editor -> Code Style -> Java -> Imports

then modify:
Class count to use import with '*' to 99
Names count to use static import with '* to 99

[mchades]

2023 -> 2024

[ajayl83]

@mchades done and thanks for settings!

[mchades]

LGTM

[mchades]

Merged, thanks for addressing the comments and improving the codebase!