alpinegizmo commented on code in PR #21349:
URL: https://github.com/apache/flink/pull/21349#discussion_r1034376565
##########
tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/optional/ShadeOptionalChecker.java:
##########
@@ -65,9 +65,23 @@ public static void main(String[] args) throws IOException {
if (!violations.isEmpty()) {
LOG.error(
- "{} modules bundle in total {} dependencies without them
being marked as optional in the pom:",
+ "{} modules bundle in total {} dependencies without them
being marked as optional in the pom.",
violations.keySet().size(),
violations.size());
+ LOG.error(
+ "\tIn order for shading to properly work within Flink we
require all bundled dependencies to be marked as optional in the pom.");
+ LOG.error(
+ "\tFor verification purposes we require the dependency
tree from the dependency-plugin to show the dependency as either:");
+ LOG.error("\t\ta) an optional dependency,");
+ LOG.error("\t\tb) a transitive dependency of another optional
dependency.");
+ LOG.error(
+ "\tIn most cases adding
'<optional>${flink.markBundledAsOptional}</optional>' to the the bundled
dependency is sufficient.");
Review Comment:
```suggestion
"\tIn most cases adding
'<optional>${flink.markBundledAsOptional}</optional>' to the bundled dependency
is sufficient.");
```
##########
tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/optional/ShadeOptionalChecker.java:
##########
@@ -65,9 +65,23 @@ public static void main(String[] args) throws IOException {
if (!violations.isEmpty()) {
LOG.error(
- "{} modules bundle in total {} dependencies without them
being marked as optional in the pom:",
+ "{} modules bundle in total {} dependencies without them
being marked as optional in the pom.",
violations.keySet().size(),
violations.size());
+ LOG.error(
+ "\tIn order for shading to properly work within Flink we
require all bundled dependencies to be marked as optional in the pom.");
+ LOG.error(
+ "\tFor verification purposes we require the dependency
tree from the dependency-plugin to show the dependency as either:");
+ LOG.error("\t\ta) an optional dependency,");
+ LOG.error("\t\tb) a transitive dependency of another optional
dependency.");
+ LOG.error(
+ "\tIn most cases adding
'<optional>${flink.markBundledAsOptional}</optional>' to the the bundled
dependency is sufficient.");
+ LOG.error(
+ "\tThere are some edge-cases where a transitive dependency
might with be associated with the \"wrong\" dependency in the tree, for example
if a test dependency also requires it.");
Review Comment:
```suggestion
"\tThere are some edge cases where a transitive
dependency might be associated with the \"wrong\" dependency in the tree, for
example if a test dependency also requires it.");
```
##########
tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/optional/ShadeOptionalChecker.java:
##########
@@ -65,9 +65,23 @@ public static void main(String[] args) throws IOException {
if (!violations.isEmpty()) {
LOG.error(
- "{} modules bundle in total {} dependencies without them
being marked as optional in the pom:",
+ "{} modules bundle in total {} dependencies without them
being marked as optional in the pom.",
violations.keySet().size(),
violations.size());
+ LOG.error(
+ "\tIn order for shading to properly work within Flink we
require all bundled dependencies to be marked as optional in the pom.");
+ LOG.error(
+ "\tFor verification purposes we require the dependency
tree from the dependency-plugin to show the dependency as either:");
+ LOG.error("\t\ta) an optional dependency,");
+ LOG.error("\t\tb) a transitive dependency of another optional
dependency.");
+ LOG.error(
+ "\tIn most cases adding
'<optional>${flink.markBundledAsOptional}</optional>' to the the bundled
dependency is sufficient.");
+ LOG.error(
+ "\tThere are some edge-cases where a transitive dependency
might with be associated with the \"wrong\" dependency in the tree, for example
if a test dependency also requires it.");
+ LOG.error(
+ "\tIn such cases you need to adjust to poms such that the
dependency shows up in the right spot; by it adding an explicit
dependency(Management) entry, excluding dependencies or at times even
reordering dependencies in the pom.");
Review Comment:
```suggestion
"\tIn such cases you need to adjust the poms so that the
dependency shows up in the right spot. This may require adding an explicit
dependency (Management) entry, excluding dependencies, or at times even
reordering dependencies in the pom.");
```
--
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]
