[GitHub] [maven-dependency-plugin] ThStock opened a new pull request #76: avoid trailing whitespace

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] ThStock opened a new pull request #76: avoid trailing whitespace

GitBox

ThStock opened a new pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76


   


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] michael-o commented on pull request #76: avoid trailing whitespace

GitBox

michael-o commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-654143960


   How can they happen?


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] michael-o edited a comment on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

michael-o edited a comment on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-654143960


   How can that happen?


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] ThStock commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

ThStock commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-655089505


   > How can that happen?
   
   It can happen if you have an " (optional) " dependecy .. see line 222 in ResolveDependenciesMojo.java


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] ThStock commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

ThStock commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-655092717


   > This could use a Jira issue explaining the bug (if it is one) and a test case demonstrating it.
   
   It is not a bug, but it will be nice if _all_ lines ends not with "{space}\n" ... 😄 as described above, it can happen with an optional dependency.


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] ThStock commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

ThStock commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-655104731


   ```
   cat pom.xml && mvn  -U dependency:tree -DoutputFile=dep.tree && sed 's, $,TRAILING_SPACE,g' dep.tree
   <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
     <groupId>test</groupId>
     <artifactId>test</artifactId>
     <version>0.1.0-SNAPSHOT</version>
     <dependencies>
       <dependency>
         <groupId>org.apache.commons</groupId>
         <artifactId>commons-lang3</artifactId>
         <version>3.10</version>
         <optional>true</optional>
       </dependency>
       <dependency>
         <groupId>org.apache.commons</groupId>
         <artifactId>commons-math3</artifactId>
         <version>3.6.1</version>
       </dependency>
     </dependencies>
     <build>
       <plugins>
         <plugin>
           <groupId>org.apache.maven.plugins</groupId>
           <artifactId>maven-dependency-plugin</artifactId>
           <version>3.1.2</version>
         </plugin>
       </plugins>
     </build>
   </project>
   [INFO] Scanning for projects...
   [INFO]
   [INFO] -----------------------------< test:test >------------------------------
   [INFO] Building test 0.1.0-SNAPSHOT
   [INFO] --------------------------------[ jar ]---------------------------------
   [INFO]
   [INFO] --- maven-dependency-plugin:3.1.2:tree (default-cli) @ test ---
   [INFO] Wrote dependency tree to: ... /dep.tree
   [INFO] ------------------------------------------------------------------------
   [INFO] BUILD SUCCESS
   [INFO] ------------------------------------------------------------------------
   [INFO] Total time:  2.877 s
   [INFO] Finished at: 2020-07-07T22:12:29+02:00
   [INFO] ------------------------------------------------------------------------
   test:test:jar:0.1.0-SNAPSHOT
   +- org.apache.commons:commons-lang3:jar:3.10:compile (optional)TRAILING_SPACE
   \- org.apache.commons:commons-math3:jar:3.6.1:compile
   ```


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] michael-o commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

michael-o commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-655330502


   You should then remove trailing space in the source and not in the target.


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] michael-o commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

michael-o commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-655432143


   But you did find the source code which produces the trailing space?


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] ThStock commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

ThStock commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-655439281


   Yes ... I'm not sure what you are trying to get from me. I'm not familiar with your project acceptance rules. I found a little thing and offered a little fix, that avoids side effects with others code.
   
   If you mean, you know all side effects and it is safe to remove the space after "..optional)" I will change it.


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] michael-o commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

michael-o commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-655448984


   I will look into the code and let you know.


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] ThStock commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

ThStock commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-658934157


   Can I help in some way?


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] michael-o commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

michael-o commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-659041083


   Completely forgot this. Please ping me by Friday.


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] ThStock commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

ThStock commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-660441146


   Ping @michael-o 😉


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] michael-o commented on pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

michael-o commented on pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76#issuecomment-661065330


   OK, went through the code, here it is:
   ```
               if ( theOutputScope && artifact.isOptional() )
               {
                   messageBuilder.a( " (optional) " );
               }
   
               // dependencies:collect won't download jars
               if ( artifact.getFile() != null )
               {
                   ModuleDescriptor moduleDescriptor = getModuleDescriptor( artifact.getFile() );
                   if ( moduleDescriptor != null )
                   {
                       messageBuilder.project( " -- module " + moduleDescriptor.name );
   
                       if ( moduleDescriptor.automatic )
                       {
                           if ( "MANIFEST".equals( moduleDescriptor.moduleNameSource ) )
                           {
                               messageBuilder.strong( " [auto]" );
                           }
                           else
                           {
                               messageBuilder.warning( " (auto)" );
                           }
                       }
                   }
               }
               artifactStringList.add( messageBuilder.toString() + System.lineSeparator() );
   ```
   
   Let's dissect and discuss:
   1. If it is not optional, we don't have a problem.
   2. If it is optional and "artifact.getFile() == null" or "moduleDescriptor == null" we'll see the trailing space. Since we'd have, in a positive case: "(optional)<SPACE><SPACE>-- module...". Why not simply remove the space from the optional? @rfscholte @hboutemy Any objections? Is my analysis correct?


----------------------------------------------------------------
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:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [maven-dependency-plugin] asfgit closed pull request #76: avoid trailing whitespace

GitBox
In reply to this post by GitBox

asfgit closed pull request #76:
URL: https://github.com/apache/maven-dependency-plugin/pull/76


   


----------------------------------------------------------------
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:
[hidden email]