Skip to content

Drop the cruft#225

Merged
olamy merged 5 commits into
masterfrom
drop-commons-collections
May 27, 2024
Merged

Drop the cruft#225
olamy merged 5 commits into
masterfrom
drop-commons-collections

Conversation

@cstamas

@cstamas cstamas commented May 27, 2024

Copy link
Copy Markdown
Member

Drop dependencies that are superfluous (one class used) or are deprecated:

  • drop commons-collections4
  • drop (to be deprecated) maven-dependency-tree

Slight updates and cleanup in dependencies.


https://issues.apache.org/jira/browse/MSHADE-473
https://issues.apache.org/jira/browse/MSHADE-474
https://issues.apache.org/jira/browse/MSHADE-475
https://issues.apache.org/jira/browse/MSHADE-476
https://issues.apache.org/jira/browse/MSHADE-477

Single class mildly used it. Simple replacement added.
@cstamas cstamas self-assigned this May 27, 2024
Comment thread pom.xml
<version>3.2.1</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>

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.

? really needed? this doesn't look related to the subject of the PR

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

? unsure I get your comment. If you'd look carefully, you will see that I just moved this dependency from last place (as it is non test) to here. I just "moved" it to have proper dep order.

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.

ah right. sorry for that.
aren't we using spotless for pom?

Comment thread pom.xml
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
<version>2.13.0</version>
<version>2.16.1</version>

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.

this doesn't look related to the subject of the PR
maybe change the subject

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

changed the subject

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.

changed the subject

thanks for that. The new subject is really more comprehensive :P

Comment thread pom.xml

<properties>
<mavenVersion>3.6.3</mavenVersion>
<mavenVersion>3.9.7</mavenVersion>

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.

this doesn't look related to the subject of the PR
maybe change the subject

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

changed the subject

Comment thread pom.xml Outdated
<groupId>org.eclipse.sisu</groupId>
<artifactId>org.eclipse.sisu.inject</artifactId>
<version>${sisu.version}</version>
<version>${version.sisu-maven-plugin}</version>

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.

the property name doesn't look related to such library.
I know it's coming from the parent but this is really confusing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, is ugly, will figure out something. My point was really to not have "disconnected" the sisu version in this plugin vs parent.

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.

Isn't this inherited anyway?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Huh, seems it is, removing whole depMgt section and moving guice version to dep, and that's did it.

@cstamas cstamas changed the title Drop commons-collections Drop cruft May 27, 2024
@cstamas cstamas changed the title Drop cruft Drop the cruft May 27, 2024

@mosabua mosabua 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.

@olamy olamy merged commit 199ffae into master May 27, 2024
@mosabua mosabua deleted the drop-commons-collections branch May 27, 2024 21:27
@olamy

olamy commented May 27, 2024

Copy link
Copy Markdown
Member

@cstamas, sorry, my bad. I merged it instead of only approving it,

@cstamas

cstamas commented May 27, 2024

Copy link
Copy Markdown
Member Author

Np, this pr was "complete", so is ok to have it merged

@jira-importer

Copy link
Copy Markdown

Resolve #528

@jira-importer

Copy link
Copy Markdown

Resolve #701

@jira-importer

Copy link
Copy Markdown

Resolve #719

@jira-importer

Copy link
Copy Markdown

Resolve #720

@jira-importer

Copy link
Copy Markdown

Resolve #725

@jira-importer

Copy link
Copy Markdown

Resolve #528

@jira-importer

Copy link
Copy Markdown

Resolve #725

@jira-importer

Copy link
Copy Markdown

Resolve #719

@jira-importer

Copy link
Copy Markdown

Resolve #701

@jira-importer

Copy link
Copy Markdown

Resolve #720

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

5 participants