XComp commented on code in PR #136:
URL: https://github.com/apache/flink-shaded/pull/136#discussion_r1466310753
##########
.github/workflows/ci.yml:
##########
@@ -49,8 +50,18 @@ jobs:
-Plicense-check \
| tee ${{ env.MVN_BUILD_OUTPUT_FILE }}
+ mvn ${MVN_COMMON_OPTIONS} dependency:tree -Plicense-check | tee ${{
env.MVN_DEPENDENCY_PLUGIN_OUTPUT_FILE }}
+
- name: Check licensing
run: |
- mvn ${MVN_COMMON_OPTIONS} exec:java@check-licensing -N \
+ mvn ${MVN_COMMON_OPTIONS} exec:java@check-license -N \
Review Comment:
nit: can we move it into its own step rather than having it included in the
Build step? ...to have a clearer separation and easier check what went wrong if
something went wrong
##########
.github/workflows/ci.yml:
##########
@@ -49,8 +50,18 @@ jobs:
-Plicense-check \
| tee ${{ env.MVN_BUILD_OUTPUT_FILE }}
+ mvn ${MVN_COMMON_OPTIONS} dependency:tree -Plicense-check | tee ${{
env.MVN_DEPENDENCY_PLUGIN_OUTPUT_FILE }}
+
- name: Check licensing
run: |
- mvn ${MVN_COMMON_OPTIONS} exec:java@check-licensing -N \
Review Comment:
That might deserve it's own hotfix commit
##########
flink-shaded-zookeeper-parent/flink-shaded-zookeeper-38/pom.xml:
##########
@@ -128,6 +131,7 @@ under the License.
<groupId>org.apache.zookeeper</groupId>
<artifactId>zookeeper</artifactId>
<version>${zookeeper.version}</version>
+ <optional>${flink.markBundledAsOptional}</optional>
</dependency>
<dependency>
<groupId>io.netty</groupId>
Review Comment:
we don't need that for BOMs?!
##########
pom.xml:
##########
@@ -65,6 +65,7 @@ under the License.
<netty.version>4.1.100.Final</netty.version>
<jackson.version>2.15.3</jackson.version>
<guava.version>32.1.3-jre</guava.version>
+ <flink.markBundledAsOptional>true</flink.markBundledAsOptional>
Review Comment:
doesn't it make sense to upgrade the shade plugin to `3.4.1` here as well
instead of having it only being upgraded for the jackson dependencies?
my understanding is (according to [your comment in
FLINK-34148](https://issues.apache.org/jira/browse/FLINK-34148?focusedCommentId=17809796&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17809796))
that the shade plugin stopped editing the dependency tree in 3.4.1.
Maven 3.3+ didn't allow editing the dependency tree anymore (based on the
description in FLINK-28016). Shouldn't we have run into issues already earlier
because we upgraded Maven in CI to 3.8.2 even though we were still using an
older version of the shade plugin? It seems that I'm missing something here.
:thinking:
##########
pom.xml:
##########
@@ -341,21 +341,41 @@ under the License.
<goals>
<goal>java</goal>
</goals>
+
+ <configuration>
+
<mainClass>org.apache.flink.tools.ci.licensecheck.LicenseChecker</mainClass>
+ </configuration>
+ </execution>
+ <execution>
+ <id>check-shade</id>
+ <!-- manually called -->
+ <phase>none</phase>
+ <goals>
+ <goal>java</goal>
+ </goals>
+
+ <configuration>
+
<mainClass>org.apache.flink.tools.ci.optional.ShadeOptionalChecker</mainClass>
+ </configuration>
</execution>
</executions>
<configuration>
-
<mainClass>org.apache.flink.tools.ci.licensecheck.LicenseChecker</mainClass>
<includePluginDependencies>true</includePluginDependencies>
<includeProjectDependencies>false</includeProjectDependencies>
</configuration>
<dependencies>
<dependency>
<groupId>org.apache.flink</groupId>
<artifactId>flink-ci-tools</artifactId>
- <version>1.17-SNAPSHOT</version>
+ <version>1.18-SNAPSHOT</version>
</dependency>
</dependencies>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
Review Comment:
why is this needed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
