Bug: Extra JARs and Artifacts were not subjected to filtering by cstamas · Pull Request #785 · apache/maven-shade-plugin (original) (raw)
Bug: the extraJars and extraArtifacts were added to shaded JAR as is, without subjecting them to filtering.
Related:
They used Java 1.5 and around as configured output, and it simply does not work with modern Java. Lifted all to Java 8.
IT should use Maven def plugin
| Shade plugin starts to create DRP even if the artifact has been renamed because of the configuration |
|---|
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switching to Java 8 feels like a separate issue. Is there some reason it's in this PR?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As ITs are busted without this change, at least I could not make ITs work locally (on Java 24, 21, etc).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Something isn't working
label
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If all ITs are busted then that should be fixed first in a separate PR.
If all ITs are busted then that should be fixed first in a separate PR.
not sure to understand the reasoning? what would be the added value of adding extra work/waste of time with a separate PR??
It has been discovered the reason of the failure with Java 25 so it's fixed now.
TBH I would like to see my "update Parent 47" draft (#781), I opend two weeks ago, to get done to work and merged, because it a) does not hide those big changes behind a "bugfix" b) sets up ITs to use parent version for lesser maintanence effort in the future (idea/request by @slawekjaranowski ). I just could not get it "green" 100% alone.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace plugin versions in ITs with parent ones (see #781)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove version. Was needed to fix Java 25 build in between, but compatible version is now part of parent
TBH I would like to see my "update Parent 47" draft (#781), I opend two weeks ago, to get done to work and merged, because it a) does not hide those big changes behind a "bugfix" b) sets up ITs to use parent version for lesser maintanence effort in the future (idea/request by @slawekjaranowski ). I just could not get it "green" 100% alone.
no worries. if you prefer this way. I was just thinking of expedite this and have it done and dusted.
Having a separate PR or not doesn't have any effect on users so up2u (it's just internal test which users doesn't know anything about)
TBH I would like to see my "update Parent 47" draft (#781), I opend two weeks ago, to get done to work and merged, because it a) does not hide those big changes behind a "bugfix" b) sets up ITs to use parent version for lesser maintanence effort in the future (idea/request by @slawekjaranowski ). I just could not get it "green" 100% alone.
no worries. if you prefer this way. I was just thinking of expedite this and have it done and dusted. Having a separate PR or not doesn't have any effect on users so up2u (it's just internal test which users doesn't know anything about)
Sure. Splitting it, would it make change history more clearer for us - the maintainers. For users of the plugin the whole parent thing doesn't matter at all. They just consume a compiled and rleased artifact.
I am about to merge this PR as:
- the actual bugfix was approved by @olamy
- the change requests by @Bukama and @elharo were about things not anymore present in this PR
I consider this PR "good to go"/
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})