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

Reject incompatible slf4j-api versions from Slf4j implementations #123

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tbroyer
Copy link

@tbroyer tbroyer commented Apr 17, 2024

Fixes #122

Copy link
Author

@tbroyer tbroyer left a comment

Choose a reason for hiding this comment

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

I haven't added tests yet (only changed a couple to make the pass), happy to discuss what should be done here (migrate everything to Slf4j 2 then add a test for the 1 vs 2 incompatibility?)

I think the approach might have to be discussed too before I spend more time on it (and then other PRs that will require Slf4j 2, e.g. for slf4j-jdk-platform-logging, or just an updated Slf4j, e.g. slf4j-reload4j).

@@ -0,0 +1,85 @@
/*
* Copyright the GradleX team.
Copy link
Author

Choose a reason for hiding this comment

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

Checkstyle currently requires that line, but I don't think this is how things work or should work, as:

  • I'm not part of the Gradlex team
  • you don't have any contributor agreement that I know of that would require copyright reassignment

Comment on lines +57 to +59
protected void additionalAdjustments(ModuleVersionIdentifier id, VariantMetadata variant) {
additionalAdjustments(variant);
}
Copy link
Author

Choose a reason for hiding this comment

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

Should this replace the overloaded method? I don't thing this should be considered a public API so introducing a breaking change like that could be acceptable.

@jjohannes jjohannes added the in:logging Things related to 'logging { }' DSL label Apr 23, 2024
@jjohannes jjohannes added a:rule One or more metadata rules that should be added or updated pending:team-discussion There is something to be discussed that blocks further progress labels May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:rule One or more metadata rules that should be added or updated in:logging Things related to 'logging { }' DSL pending:team-discussion There is something to be discussed that blocks further progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Patch logback >= 1.3 to require slf4j >= 2.0, logback < 1.3 to require slf4j <= 1.7.x
2 participants