You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by GitBox <gi...@apache.org> on 2021/02/20 20:28:03 UTC
[GitHub] [storm] bipinprasad opened a new pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
bipinprasad opened a new pull request #3380:
URL: https://github.com/apache/storm/pull/3380
## What is the purpose of the change
*Shaded classes are not visible within IntelliJ IDE*
## How was the change tested
*Debug a test class in storm-server within the IntelliJ IDE*
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on pull request #3380:
URL: https://github.com/apache/storm/pull/3380#issuecomment-800610561
This works nicely for this particular purpose within Intelij IDE.
But is there any side effect?
From the description, https://www.mojohaus.org/build-helper-maven-plugin/attach-artifact-mojo.html, it says "Attach additional artifacts to be installed and deployed."
I don't fully understand the meaning of it. Will this attached artifact be deployed with `mvn install` or `mvn deploy`?
With `mvn install`, I saw
```
[INFO] --- maven-install-plugin:2.5.2:install (default-install) @ storm-shaded-deps ---
[INFO] Installing /Users/ethanli/Workspace/community/storm/storm-shaded-deps/target/storm-shaded-deps-2.3.0-SNAPSHOT.jar to /Users/ethanli/.m2/repository/org/apache/storm/storm-shaded-deps/2.3.0-SNAPSHOT/storm-shaded-deps-2.3.0-SNAPSHOT.jar
[INFO] Installing /Users/ethanli/Workspace/community/storm/storm-shaded-deps/dependency-reduced-pom.xml to /Users/ethanli/.m2/repository/org/apache/storm/storm-shaded-deps/2.3.0-SNAPSHOT/storm-shaded-deps-2.3.0-SNAPSHOT.pom
[INFO] Installing /Users/ethanli/Workspace/community/storm/storm-shaded-deps/target/storm-shaded-deps-2.3.0-SNAPSHOT.jar to /Users/ethanli/.m2/repository/org/apache/storm/storm-shaded-deps/2.3.0-SNAPSHOT/storm-shaded-deps-2.3.0-SNAPSHOT-optional.jar
```
that this storm-shaded-deps-2.3.0-SNAPSHOT.jar is installed twice into the local maven repo. I wonder if it will upload two copies of it to remote maven repo during `mvn deploy`.
I feel this side effect is not really desired.
I usually solve this intelij problem by this workaround: https://youtrack.jetbrains.com/issue/IDEA-126596#focus=Comments-27-757181.0-0
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3380:
URL: https://github.com/apache/storm/pull/3380#discussion_r600705948
##########
File path: storm-shaded-deps/pom.xml
##########
@@ -297,6 +297,43 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
</plugin>
+ <plugin>
+ <!--
+ Purpose of build-helper-maven-plugin is to ensure that IntelliJ sees shaded classes.
+ When navigating over to java classes in storm-client module, the shaded imported classes will
+ show as resolved instead of unknown.
+ The artifact specified below is created when this module (storm-shaded-deps) is built.
+ But is also used to resolve classes in IntelliJ.
+ "phase" is set to "none" - with this setting, the artifact jar will not be uploaded to repo
+ when running standalone "mvn install" command.
+
+ (DO NOT DO THIS) If "phase" is changed to "package" then standalone "mvn install" command will upload
+ this very same artifact as storm-shaded-deps-${project.version}-optional.jar - which is not desired.
+
+ build-helper-maven-plugin can be removed completely with no effect on the "mvn clean" and "mvn install",
+ which is handled by mvn-shade-plugin.
Review comment:
nit: This should be `maven-shade-plugin`
Otherwise looks good to me.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] bipinprasad commented on a change in pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
bipinprasad commented on a change in pull request #3380:
URL: https://github.com/apache/storm/pull/3380#discussion_r598769627
##########
File path: storm-shaded-deps/pom.xml
##########
@@ -297,6 +297,28 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>build-helper-maven-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>compile</id>
+ <phase>none</phase>
+ <goals>
+ <goal>attach-artifact</goal>
+ </goals>
+ <configuration>
+ <artifacts>
+ <artifact>
+ <file>${basedir}/target/storm-shaded-deps-${project.version}.jar</file>
Review comment:
Never mind - this works too. Those variables are defined in pom-4.0.0.xml.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] bipinprasad commented on a change in pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
bipinprasad commented on a change in pull request #3380:
URL: https://github.com/apache/storm/pull/3380#discussion_r598759843
##########
File path: storm-shaded-deps/pom.xml
##########
@@ -297,6 +297,28 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>build-helper-maven-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>compile</id>
Review comment:
This change works.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] bipinprasad commented on a change in pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
bipinprasad commented on a change in pull request #3380:
URL: https://github.com/apache/storm/pull/3380#discussion_r598751952
##########
File path: storm-shaded-deps/pom.xml
##########
@@ -297,6 +297,28 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>build-helper-maven-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>compile</id>
Review comment:
Will test this change. I thought it had to be be one of the standard ids.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] Ethanlm merged pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
Ethanlm merged pull request #3380:
URL: https://github.com/apache/storm/pull/3380
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] bipinprasad commented on a change in pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
bipinprasad commented on a change in pull request #3380:
URL: https://github.com/apache/storm/pull/3380#discussion_r598751559
##########
File path: storm-shaded-deps/pom.xml
##########
@@ -297,6 +297,28 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>build-helper-maven-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>compile</id>
+ <phase>none</phase>
+ <goals>
+ <goal>attach-artifact</goal>
+ </goals>
+ <configuration>
+ <artifacts>
+ <artifact>
+ <file>${basedir}/target/storm-shaded-deps-${project.version}.jar</file>
Review comment:
This jar file has to be the one that was built (using the shade plugin). Can't be an arbitrary or unavailable jar AFAIK.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] bipinprasad commented on a change in pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
bipinprasad commented on a change in pull request #3380:
URL: https://github.com/apache/storm/pull/3380#discussion_r598751952
##########
File path: storm-shaded-deps/pom.xml
##########
@@ -297,6 +297,28 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>build-helper-maven-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>compile</id>
Review comment:
Will test this change.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3380:
URL: https://github.com/apache/storm/pull/3380#discussion_r600705948
##########
File path: storm-shaded-deps/pom.xml
##########
@@ -297,6 +297,43 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
</plugin>
+ <plugin>
+ <!--
+ Purpose of build-helper-maven-plugin is to ensure that IntelliJ sees shaded classes.
+ When navigating over to java classes in storm-client module, the shaded imported classes will
+ show as resolved instead of unknown.
+ The artifact specified below is created when this module (storm-shaded-deps) is built.
+ But is also used to resolve classes in IntelliJ.
+ "phase" is set to "none" - with this setting, the artifact jar will not be uploaded to repo
+ when running standalone "mvn install" command.
+
+ (DO NOT DO THIS) If "phase" is changed to "package" then standalone "mvn install" command will upload
+ this very same artifact as storm-shaded-deps-${project.version}-optional.jar - which is not desired.
+
+ build-helper-maven-plugin can be removed completely with no effect on the "mvn clean" and "mvn install",
+ which is handled by mvn-shade-plugin.
Review comment:
This should be `maven-shade-plugin`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3380:
URL: https://github.com/apache/storm/pull/3380#discussion_r596209831
##########
File path: storm-shaded-deps/pom.xml
##########
@@ -297,6 +297,28 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>build-helper-maven-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>compile</id>
+ <phase>none</phase>
+ <goals>
+ <goal>attach-artifact</goal>
+ </goals>
+ <configuration>
+ <artifacts>
+ <artifact>
+ <file>${basedir}/target/storm-shaded-deps-${project.version}.jar</file>
Review comment:
Maybe use `<file>${project.build.directory}/${project.build.finalName}.jar</file>` ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3380: [STORM-3744] Change pom.xml so that shaded classes are visible within IntelliJ IDE
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3380:
URL: https://github.com/apache/storm/pull/3380#discussion_r596221955
##########
File path: storm-shaded-deps/pom.xml
##########
@@ -297,6 +297,28 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>build-helper-maven-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>compile</id>
Review comment:
Maybe change the id to something like `<id>workaround-makeItVisibleOnIntellij</id>` since the ID here is only informational, to better explain the purpose.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org