Skip to content

[HIVE-28794]: Remove junit version from hive-metastore-tools/pom.xml to inherit version from parent pom - #5668

Merged
ayushtkn merged 1 commit into
apache:masterfrom
arorasimran0309:junit_upgrd
Apr 2, 2025
Merged

[HIVE-28794]: Remove junit version from hive-metastore-tools/pom.xml to inherit version from parent pom#5668
ayushtkn merged 1 commit into
apache:masterfrom
arorasimran0309:junit_upgrd

Conversation

@arorasimran0309

@arorasimran0309 arorasimran0309 commented Mar 3, 2025

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Junit version upgrade in hive-metastore-tools

Why are the changes needed?

To bring version consistency throughout

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

Yes

How was this patch tested?

Before:
image
image

After:
image
image

dptree_junit.txt

<junit.jupiter.version>5.6.2</junit.jupiter.version>
<junit.vintage.version>5.6.2</junit.vintage.version>
<junit.jupiter.version>5.11.2</junit.jupiter.version>
<junit.vintage.version>5.11.2</junit.vintage.version>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The intention looks good to me. I wonder why we have to specify the version in both standalone-metastore/pom.xml and this file. Can we remove these properties and inherit the dependency management settings of the parent project?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

long long ago, there was a initiative HIVE-17159, where people wanted to make sure standalone-metastore should be independently releasable, so this is the reason these versions are duped here.

Though that initiative didn't work the way people wanted, but did leave this for us & I think going back has another set of challenges, considering it had consensus back then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation. In this case, we replicate the same property four times. I feel that the property in standalone-metastore/metastore-tools/pom.xml can be consolidated to that in standalone-metastore/pom.xml.

% git grep '<junit.jupiter.version>'
pom.xml:    <junit.jupiter.version>5.11.2</junit.jupiter.version>
standalone-metastore/metastore-tools/pom.xml:    <junit.jupiter.version>5.6.2</junit.jupiter.version>
standalone-metastore/pom.xml:    <junit.jupiter.version>5.11.2</junit.jupiter.version>
storage-api/pom.xml:    <junit.jupiter.version>5.11.2</junit.jupiter.version>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review and suggestions. Made the necessary changes.
<junit.jupiter.version> and <junit.vintage.version> consolidated, to be inherited from the parent standalone-metastore/pom.xml

@arorasimran0309 arorasimran0309 changed the title [HIVE-28794]: Upgrade junit version for hive-metastore-tools [HIVE-28794]: Remove junit version from hive-metastore-tools/pom.xml to inherit version from parent pom Mar 28, 2025

@ayushtkn ayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

    <junit.platform.runner.version>1.2.0</junit.platform.runner.version>
    <junit.jupiter.api.version>5.2.0</junit.jupiter.api.version>

These are also defined in this pom, I don't think they are being used, can we drop them as well?

@sonarqubecloud

Copy link
Copy Markdown

@arorasimran0309

Copy link
Copy Markdown
Contributor Author
    <junit.platform.runner.version>1.2.0</junit.platform.runner.version>
    <junit.jupiter.api.version>5.2.0</junit.jupiter.api.version>

These are also defined in this pom, I don't think they are being used, can we drop them as well?

Yes, the version specifications <junit.platform.runner.version> and <junit.jupiter.api.version> are orphaned, and therefore can be dropped. Have made the required changes to clean up the codebase.

@ayushtkn ayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ayushtkn
ayushtkn merged commit 95db41c into apache:master Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants