-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
upgrade maven-shade-plugin version to 3.5.2 #12712
upgrade maven-shade-plugin version to 3.5.2 #12712
Conversation
34c0fbc
to
07f55c5
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #12712 +/- ##
============================================
+ Coverage 61.75% 62.17% +0.42%
+ Complexity 207 198 -9
============================================
Files 2436 2502 +66
Lines 133233 136486 +3253
Branches 20636 21122 +486
============================================
+ Hits 82274 84866 +2592
- Misses 44911 45357 +446
- Partials 6048 6263 +215
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
07f55c5
to
4eddca6
Compare
bba0e19
to
1e727fc
Compare
1e727fc
to
0c8d28c
Compare
79a56b2
to
15b556f
Compare
d4a892a
to
736360f
Compare
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.
Nice!
@@ -214,7 +214,6 @@ | |||
<plugin> | |||
<groupId>org.apache.maven.plugins</groupId> | |||
<artifactId>maven-shade-plugin</artifactId> | |||
<version>3.1.0</version> |
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.
Do we need to keep this plugin? I guess we should never package pinot-perf?
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.
this is not packaged to pinot-distribution, it's trying to generate another main class for perf testing.
@@ -35,6 +35,6 @@ | |||
<url>https://pinot.apache.org/</url> | |||
<properties> | |||
<pinot.root>${basedir}/../../..</pinot.root> | |||
<phase.prop>none</phase.prop> | |||
<shade.phase.prop>none</shade.phase.prop> |
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.
(minor) Not needed?
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.
true
<id>build-shaded-jar</id> | ||
<activation> | ||
<property> | ||
<name>skipShade</name> |
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.
Do we want to keep this option in the main pom?
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.
I don't see the usage for this for a while.
@@ -2209,6 +2211,9 @@ | |||
<!-- Exclude build targets --> | |||
<exclude>**/target/**</exclude> | |||
|
|||
<!-- Exclude Maven plugin generated files --> | |||
<exclude>**/dependency-reduced-pom.xml</exclude> |
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.
I never saw this file generated
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.
new version plugin generates this file
736360f
to
2e74761
Compare
<profiles> | ||
<profile> | ||
<id>build-shaded-jar</id> | ||
<activation> | ||
<activeByDefault>false</activeByDefault> | ||
</activation> |
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.
After removing this these module level profile the mvn build stopped to publish shaded jars for spi which is required by our build pipeline. We should probably add it back.
Upgrade maven-shade-plugin version from 3.2.x to 3.5.2.
By default, modules are not building the shaded jar.
For the modules require shaded jars, users can specify
<shade.phase.prop>package</shade.phase.prop>
in thepom.xml
fileproperties
section.