You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2021/05/14 11:11:45 UTC

[GitHub] [zeppelin] cuspymd opened a new pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

cuspymd opened a new pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118


   ### What is this PR for?
   Polish pom.xml files
   - Delete unnecessary tags
   - Delete duplicated dependencies 
   
   
   ### What type of PR is it?
   [Refactoring]
   
   ### What is the Jira issue?
   * https://issues.apache.org/jira/browse/ZEPPELIN-5370
   
   ### How should this be tested?
   * CI
   
   
   ### Questions:
   * Does the licenses files need update? No
   * Is there breaking changes for older versions? No
   * Does this needs documentation? No
   


-- 
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] [zeppelin] Reamer commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r634925921



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>

Review comment:
       You are right, I changed my mind because of the good xsd.
   Defaults should not be declared, so you should remove this.




-- 
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] [zeppelin] Reamer commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-871168481


   I am very interested in this PR. @cuspymd Thank you for your work.
   Should we also backport this to branch-0.9?


-- 
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: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] cuspymd commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
cuspymd commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-875252940


   > 
   > 
   > I am very interested in this PR. @cuspymd Thank you for your work.
   > Should we also backport this to branch-0.9?
   
   Are there any benefits to backporting it to branch-0.9?
   And, at the time of rebase, there were conflicts with the modifications of @zjffdu, 
   so it was difficult to merge. Backporting only these fixes would not be easy.


-- 
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: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] zjffdu commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-859234456


   @cuspymd Could you check the failed spark integration test ?


-- 
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] [zeppelin] zjffdu commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-873395992


   ping @cuspymd 


-- 
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: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] Reamer commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r634938512



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>
     </parent>
 
-    <groupId>org.apache.zeppelin</groupId>

Review comment:
       The r module is deprecated.
   https://github.com/apache/zeppelin/commit/b62c6078d1d6dee30f9e35e10bf7e6d0f1e1ffc5




-- 
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] [zeppelin] zjffdu commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-863723489


   ping @cuspymd 


-- 
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] [zeppelin] cuspymd commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r634479834



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>

Review comment:
       As `maven-4.0.0.xsd`, The default value of the tag `relativePath` is "../pom.xml". Nevertheless, do you think it should be declared explicitly?
   > The relative path of the parent <code>pom.xml</code> file within the check out. If not specified, it defaults to <code>../pom.xml</code>.
   > https://maven.apache.org/xsd/maven-4.0.0.xsd




-- 
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] [zeppelin] cuspymd commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
cuspymd commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-871081021


   > 
   > 
   > ping @cuspymd
   
   I fixed it and rebased codes


-- 
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: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] Reamer commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-856506555


   @cuspymd Please rebase to current master to fix CI.


-- 
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] [zeppelin] zjffdu commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
zjffdu commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-841590906


   Seems 2 CI fails @cuspymd 


-- 
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] [zeppelin] Reamer commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r656118380



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>
     </parent>
 
-    <groupId>org.apache.zeppelin</groupId>

Review comment:
       PR #4145 deletes the R module.




-- 
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] [zeppelin] Reamer commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r634951058



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>
     </parent>
 
-    <groupId>org.apache.zeppelin</groupId>

Review comment:
       Maybe it's time to remove the module.
   https://github.com/Reamer/zeppelin/commit/1298e2619d168647806c09a913a69a5bc8883c8b
   @zjffdu what are your thinking?




-- 
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] [zeppelin] cuspymd commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r634501953



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>
     </parent>
 
-    <groupId>org.apache.zeppelin</groupId>

Review comment:
       I found the version value of "r/pom.xml" is "0.9.0-SNAPSHOT". Can I understand that it is missing from the version update changes?




-- 
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] [zeppelin] cuspymd commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r634501953



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>
     </parent>
 
-    <groupId>org.apache.zeppelin</groupId>

Review comment:
       Upon checking, only the version value of "r/pom.xml" is "0.9.0-SNAPSHOT". Can I understand that it is missing from the version update changes?




-- 
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] [zeppelin] Reamer commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r634058596



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>

Review comment:
       I think `relativePath` should still be used at this point, but it should point to the pom.xml.
   ```
   <relativePath>../pom.xml</relativePath>
   ```
   I know that the parent directory is the default and can be omitted, but sometimes you don't run Maven over the whole project, like in our [tests](https://github.com/apache/zeppelin/blob/97ce203a7fb72fd0300ff84ea5fa8e52f6125ebc/.github/workflows/core.yml#L67), then we should reference the parent project with a relative path.
   
   > However, that would work if the parent project was already installed in our local repository or was in that specific directory structure (parent pom.xml is one directory higher than that of the module's pom.xml).
   https://maven.apache.org/guides/introduction/introduction-to-the-pom.html#Project_Inheritance
   

##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>
     </parent>
 
-    <groupId>org.apache.zeppelin</groupId>

Review comment:
       When you clean up the pom files, you should also delete the versions.
   
   >Alternatively, if you want the groupId or the version of your modules to be the same as their parents, you can remove the groupId or the version identity of your module in its POM.
   https://maven.apache.org/guides/introduction/introduction-to-the-pom.html#Project_Inheritance




-- 
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] [zeppelin] Reamer commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-851513784


   @cuspymd Please rebase to current master to fix CI.


-- 
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] [zeppelin] Reamer commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-875582690


   You are right, I have merged this into Master.


-- 
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: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zeppelin] cuspymd commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
cuspymd commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-852172380


   > @cuspymd Please rebase to current master to fix CI.
   
   I have done.


-- 
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] [zeppelin] Reamer commented on pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#issuecomment-845220748


   LGTM


-- 
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] [zeppelin] cuspymd commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r634479834



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>

Review comment:
       The default value of the tag `relativePath` is "../pom.xml". Nevertheless, do you think it should be declared explicitly?
   > The relative path of the parent <code>pom.xml</code> file within the check out. If not specified, it defaults to <code>../pom.xml</code>.
   > https://maven.apache.org/xsd/maven-4.0.0.xsd




-- 
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] [zeppelin] Reamer commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
Reamer commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r658532497



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>
     </parent>
 
-    <groupId>org.apache.zeppelin</groupId>

Review comment:
       @cuspymd I removed the deprecated r module, please rebase your PR.




-- 
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] [zeppelin] cuspymd commented on a change in pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
cuspymd commented on a change in pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118#discussion_r636204724



##########
File path: zeppelin-plugins/pom.xml
##########
@@ -25,10 +25,8 @@
         <artifactId>zeppelin</artifactId>
         <groupId>org.apache.zeppelin</groupId>
         <version>0.10.0-SNAPSHOT</version>
-        <relativePath>..</relativePath>
     </parent>
 
-    <groupId>org.apache.zeppelin</groupId>

Review comment:
       I have deleted duplicated version tags of sub modules




-- 
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] [zeppelin] asfgit closed pull request #4118: [ZEPPELIN-5370] Polish pom.xml files

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4118:
URL: https://github.com/apache/zeppelin/pull/4118


   


-- 
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: dev-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org