You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/06/27 11:17:07 UTC

[GitHub] [maven-file-management] cstamas opened a new pull request, #11: [MSHARED-1090] Update module

cstamas opened a new pull request, #11:
URL: https://github.com/apache/maven-file-management/pull/11

   Update module:
   * parent to POM 36
   * Maven level to 3.2.5
   * Java level to 8
   * drop m-shared-utils
   * drop legacy APIs like plexus logger
   
   ---
   
   https://issues.apache.org/jira/browse/MSHARED-1090


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1169190109

   Tidied up completely


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] michael-o commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1167332564

   Why do you I have the feeling that I have seen this with Plexus Utils too?!


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1167272364

   Of course, but did not want to mix the two (there are a LOT of other changes already requiring 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.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1167304592

   Hm, maybe we need NoopMessageSink to retain existing behaviour with defautl ctor... as now it will use sl4j logger and this is behaviour change from before when def ctor used.


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1167322093

   maven-jar-plugin for sure, unsure for others, but gh search reveals a LOT of uses across many (non our) plugins
   


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1167323621

   https://github.com/search?l=Java&q=org.apache.maven.shared.model.fileset.util.FileSetManager&type=Code


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] jorsol commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
jorsol commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1167269448

   This change should probably bump the minor version.


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on a diff in pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#discussion_r908930158


##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -61,77 +63,31 @@
     /**
      * Create a new manager instance with the supplied log instance and flag for whether to output verbose messages.
      *
-     * @param log the mojo log instance
+     * @param logger the logger instance
      * @param verbose whether to output verbose messages
      */
-    public FileSetManager( Log log, boolean verbose )
-    {
-        if ( verbose )
-        {
-            this.messages =
-                new MessageHolder( MessageLevels.LEVEL_DEBUG, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        }
-        else
-        {
-            this.messages =
-                new MessageHolder( MessageLevels.LEVEL_INFO, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        }
-
-        this.verbose = verbose;
-    }
-
-    /**
-     * Create a new manager instance with the supplied log instance. Verbose flag is set to false.
-     *
-     * @param log The mojo log instance
-     */
-    public FileSetManager( Log log )
+    public FileSetManager( Logger logger, boolean verbose )
     {
-        this.messages =
-            new MessageHolder( MessageLevels.LEVEL_INFO, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        this.verbose = false;
-    }
-
-    /**
-     * Create a new manager instance with the supplied log instance and flag for whether to output verbose messages.
-     *
-     * @param log The mojo log instance
-     * @param verbose Whether to output verbose messages
-     */
-    public FileSetManager( Logger log, boolean verbose )
-    {
-        if ( verbose )
-        {
-            this.messages = new MessageHolder( MessageLevels.LEVEL_DEBUG, MessageLevels.LEVEL_INFO,
-                                                      new PlexusLoggerSink( log ) );
-        }
-        else
-        {
-            this.messages = new MessageHolder( MessageLevels.LEVEL_INFO, MessageLevels.LEVEL_INFO,
-                                                      new PlexusLoggerSink( log ) );
-        }
-
+        this.logger = requireNonNull( logger );
         this.verbose = verbose;
     }
 
     /**
      * Create a new manager instance with the supplied log instance. Verbose flag is set to false.
      *
-     * @param log The mojo log instance
+     * @param logger The log instance
      */
-    public FileSetManager( Logger log )
+    public FileSetManager( Logger logger )

Review Comment:
   Disagree, this ctor controls what logger this class will use.



-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1169218949

   General: this is a utility class, is NOT a component.


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas merged pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas merged PR #11:
URL: https://github.com/apache/maven-file-management/pull/11


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] michael-o commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1167309543

   Where is this module still used?


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1169217153

   re verbose: disagree. This is "verbose", will tell more, that's all. Log level is INFO


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on a diff in pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#discussion_r908929943


##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -61,77 +63,31 @@
     /**
      * Create a new manager instance with the supplied log instance and flag for whether to output verbose messages.
      *
-     * @param log the mojo log instance
+     * @param logger the logger instance
      * @param verbose whether to output verbose messages
      */
-    public FileSetManager( Log log, boolean verbose )
-    {
-        if ( verbose )
-        {
-            this.messages =
-                new MessageHolder( MessageLevels.LEVEL_DEBUG, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        }
-        else
-        {
-            this.messages =
-                new MessageHolder( MessageLevels.LEVEL_INFO, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        }
-
-        this.verbose = verbose;
-    }
-
-    /**
-     * Create a new manager instance with the supplied log instance. Verbose flag is set to false.
-     *
-     * @param log The mojo log instance
-     */
-    public FileSetManager( Log log )
+    public FileSetManager( Logger logger, boolean verbose )

Review Comment:
   Disagree, this ctor controls what logger this class will use and verbosity (not level)



-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1169217540

   verbose is orthogonal to log level. That's all IMHO


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] slawekjaranowski commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1168913935

   As @slachiewicz said - why not use slf4j?
   
   There is changed from plexus Logger to slf4j Logger so change will be incompatible.


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] slachiewicz commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
slachiewicz commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1167390384

   Maybe we can just switch to pure slf4j?


-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] michael-o commented on a diff in pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#discussion_r908819988


##########
pom.xml:
##########
@@ -71,24 +72,36 @@
       <groupId>org.apache.maven</groupId>
       <artifactId>maven-plugin-api</artifactId>
       <version>${mavenVersion}</version>
+      <scope>provided</scope>
     </dependency>
-
     <dependency>
-      <groupId>org.apache.maven.shared</groupId>
-      <artifactId>maven-shared-utils</artifactId>
-      <version>3.3.3</version>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+      <version>${slf4jVersion}</version>
     </dependency>
 
     <dependency>
       <groupId>org.codehaus.plexus</groupId>
       <artifactId>plexus-utils</artifactId>
-      <version>3.3.0</version>
+      <version>3.4.2</version>
+    </dependency>
+    <dependency>
+      <groupId>commons-io</groupId>
+      <artifactId>commons-io</artifactId>
+      <version>2.11.0</version>
     </dependency>
+
     <!-- Test -->
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
-      <version>4.13.1</version>
+      <version>4.13.2</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-nop</artifactId>

Review Comment:
   Really nop or simple?



-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] michael-o commented on a diff in pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#discussion_r908924294


##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -61,77 +63,31 @@
     /**
      * Create a new manager instance with the supplied log instance and flag for whether to output verbose messages.
      *
-     * @param log the mojo log instance
+     * @param logger the logger instance
      * @param verbose whether to output verbose messages
      */
-    public FileSetManager( Log log, boolean verbose )
-    {
-        if ( verbose )
-        {
-            this.messages =
-                new MessageHolder( MessageLevels.LEVEL_DEBUG, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        }
-        else
-        {
-            this.messages =
-                new MessageHolder( MessageLevels.LEVEL_INFO, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        }
-
-        this.verbose = verbose;
-    }
-
-    /**
-     * Create a new manager instance with the supplied log instance. Verbose flag is set to false.
-     *
-     * @param log The mojo log instance
-     */
-    public FileSetManager( Log log )
+    public FileSetManager( Logger logger, boolean verbose )

Review Comment:
   I think this ctor should be deprecated.



##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -285,18 +241,18 @@ public void delete( FileSet fileSet, boolean throwsError )
                 {
                     if ( fileSet.isFollowSymlinks() || !isSymlink( file ) )
                     {
-                        if ( verbose && messages != null )
+                        if ( verbose )

Review Comment:
   This completely circumvents log levels



##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -380,9 +336,9 @@ private Set<String> findDeletablePaths( FileSet fileSet )
 
     private Set<String> findDeletableDirectories( FileSet fileSet )
     {
-        if ( verbose && messages != null )
+        if ( verbose )

Review Comment:
   ditto



##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -455,31 +411,31 @@ private Set<String> findDeletableFiles( FileSet fileSet, Set<String> deletableDi
 
         if ( !fileSet.isFollowSymlinks() )
         {
-            if ( verbose && messages != null )
+            if ( verbose )

Review Comment:
   ditto



##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -61,77 +63,31 @@
     /**
      * Create a new manager instance with the supplied log instance and flag for whether to output verbose messages.
      *
-     * @param log the mojo log instance
+     * @param logger the logger instance
      * @param verbose whether to output verbose messages
      */
-    public FileSetManager( Log log, boolean verbose )
-    {
-        if ( verbose )
-        {
-            this.messages =
-                new MessageHolder( MessageLevels.LEVEL_DEBUG, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        }
-        else
-        {
-            this.messages =
-                new MessageHolder( MessageLevels.LEVEL_INFO, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        }
-
-        this.verbose = verbose;
-    }
-
-    /**
-     * Create a new manager instance with the supplied log instance. Verbose flag is set to false.
-     *
-     * @param log The mojo log instance
-     */
-    public FileSetManager( Log log )
+    public FileSetManager( Logger logger, boolean verbose )
     {
-        this.messages =
-            new MessageHolder( MessageLevels.LEVEL_INFO, MessageLevels.LEVEL_INFO, new MojoLogSink( log ) );
-        this.verbose = false;
-    }
-
-    /**
-     * Create a new manager instance with the supplied log instance and flag for whether to output verbose messages.
-     *
-     * @param log The mojo log instance
-     * @param verbose Whether to output verbose messages
-     */
-    public FileSetManager( Logger log, boolean verbose )
-    {
-        if ( verbose )
-        {
-            this.messages = new MessageHolder( MessageLevels.LEVEL_DEBUG, MessageLevels.LEVEL_INFO,
-                                                      new PlexusLoggerSink( log ) );
-        }
-        else
-        {
-            this.messages = new MessageHolder( MessageLevels.LEVEL_INFO, MessageLevels.LEVEL_INFO,
-                                                      new PlexusLoggerSink( log ) );
-        }
-
+        this.logger = requireNonNull( logger );
         this.verbose = verbose;
     }
 
     /**
      * Create a new manager instance with the supplied log instance. Verbose flag is set to false.
      *
-     * @param log The mojo log instance
+     * @param logger The log instance
      */
-    public FileSetManager( Logger log )
+    public FileSetManager( Logger logger )

Review Comment:
   I think this ctor should be deprecated.



##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -398,31 +354,31 @@ private Set<String> findDeletableDirectories( FileSet fileSet )
 
         if ( !fileSet.isFollowSymlinks() )
         {
-            if ( verbose && messages != null )
+            if ( verbose )

Review Comment:
   ditto



##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -437,9 +393,9 @@ private Set<String> findDeletableDirectories( FileSet fileSet )
 
     private Set<String> findDeletableFiles( FileSet fileSet, Set<String> deletableDirectories )
     {
-        if ( verbose && messages != null )
+        if ( verbose )

Review Comment:
   ditto



##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -316,9 +272,9 @@ public void delete( FileSet fileSet, boolean throwsError )
                 }
                 else
                 {
-                    if ( verbose && messages != null )
+                    if ( verbose )

Review Comment:
   ditto



##########
src/main/java/org/apache/maven/shared/model/fileset/util/FileSetManager.java:
##########
@@ -285,18 +241,18 @@ public void delete( FileSet fileSet, boolean throwsError )
                 {
                     if ( fileSet.isFollowSymlinks() || !isSymlink( file ) )
                     {
-                        if ( verbose && messages != null )
+                        if ( verbose )
                         {
-                            messages.addInfoMessage( "Deleting directory: " + file ).flush();
+                            logger.info( "Deleting directory: " + file );
                         }
 
                         removeDir( file, fileSet.isFollowSymlinks(), throwsError, warnMessages );
                     }
                     else
                     { // delete a symlink to a directory without follow
-                        if ( verbose && messages != null )
+                        if ( verbose )

Review Comment:
   ditto



-- 
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: issues-unsubscribe@maven.apache.org

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


[GitHub] [maven-file-management] cstamas commented on pull request #11: [MSHARED-1090] Update module

Posted by GitBox <gi...@apache.org>.
cstamas commented on PR #11:
URL: https://github.com/apache/maven-file-management/pull/11#issuecomment-1168588059

   > Why do you I have the feeling that I have seen this with Plexus Utils too?!
   
   This is "already seen" to me as well, no idea where but really looks familiar...


-- 
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: issues-unsubscribe@maven.apache.org

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