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

Refactor/replace plexus abstract log enabled with slf4j #1917

Conversation

timtebeek
Copy link
Contributor

As seen on https://cwiki.apache.org/confluence/plugins/servlet/mobile?contentId=181305684#MavenEcosystemCleanup-DropPlexusContainerforJSR-330+SisuGuiceextension

-Leverages openrewrite/rewrite-apache#41

This was the last one of this series; wasn't too sure about removal of extends AbstractLogEnabled from the deprecated classes; applied now to the classes that were not explicitly backwards compatibility tests, and retained in the two cases that were.

@elharo could you also look over this last one? That would mostly conclude the removal except where not yet possible.

@timtebeek
Copy link
Contributor Author

Thanks for the quick review! Looks to have been a one off failure in CI:

Caused by: java.net.ConnectException: HTTP connect timed out

@gnodet
Copy link
Contributor

gnodet commented Nov 17, 2024

What's the benefits of refactoring deprecated classes ?

@timtebeek
Copy link
Contributor Author

Hmm; you're right. I only just now noticed all of the classes here are in compat, as opposed to just the few I'd reverted when tests broke on the changed backwards compatibility. Guess that means there's little sense in changing this now; luckily the changes were mostly automated. Thanks for pointing it out!

I did already have my eye on completely removing maven-compat and was considering automation in that area next. Is there any set of steps on what would be required? Would this better fit a Slack or mailling list conversation?

@elharo
Copy link
Contributor

elharo commented Nov 17, 2024

Many of our "deprecated" classes are still in active use.

@timtebeek timtebeek closed this Nov 17, 2024
@timtebeek
Copy link
Contributor Author

Many of our "deprecated" classes are still in active use.

I'd closed the PR before seeing this comment; I'll leave the branch up in case you'd want to reopen and merge still.

@gnodet
Copy link
Contributor

gnodet commented Nov 17, 2024

I did already have my eye on completely removing maven-compat and was considering automation in that area next. Is there any set of steps on what would be required? Would this better fit a Slack or mailling list conversation?

maven-compat is not really used anymore in Maven core project anymore, the problem rely more in other plugins.
See #1487

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants