Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Chore/upgrade spring to 5 3 #216
base: master
Are you sure you want to change the base?
Chore/upgrade spring to 5 3 #216
Changes from 5 commits
3f2c8bd
8e76f03
763db07
ef58af8
5fd2c58
3c626d9
5002fc7
7ed1fc4
29d42d8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this removed? When I deploy the portlet to Tomcat, I see both these jars in WEB-INF/lib, but I think we only want the slf4j one:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For logging, the goal is logback (via the slf4j api). All other logging implementations (even transitive ones) should be removed. I'll look into how commons-logging is still getting deployed.
The JCL bridges (-over-) are now included in spring-jcl, so we don't have to explicitly declare them in the pom, however, I'll need to check if
jcl-over-slf4j-1.7.5.jar
is from spring-jcl, or a transitive dependency that should be excluded.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, lines 201-206 actually prevent any dependency from pulling in commons-logging, which is why I think it should remain. Otherwise, you have to find which dependencies are pulling in commons-logging and add excludes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per our side discussion with @bjagg , I'm going in the direction of using the maven enforcer plugin, and then excluding the commons-logging jars as needed. @groybal - please let me know if you don't feel this is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
discussed in PR - change the parent pom to have a
dependencies
section with the excluded deps as 'provided'