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/10 14:37:53 UTC
[GitHub] [maven-filtering] michael-o commented on a diff in pull request #38: [MSHARED-1080] Parent POM 36, Java8, drop legacy.
michael-o commented on code in PR #38:
URL: https://github.com/apache/maven-filtering/pull/38#discussion_r894589012
##########
pom.xml:
##########
@@ -55,7 +56,9 @@
<properties>
<mavenVersion>3.2.5</mavenVersion>
- <javaVersion>7</javaVersion>
+ <slf4jVersion>1.7.36</slf4jVersion>
+ <plexusBuildApiVersion>0.0.7</plexusBuildApiVersion>
+ <javaVersion>8</javaVersion>
Review Comment:
Please retain the position of `javaVersion`, makes easier to diff.
##########
src/main/java/org/apache/maven/shared/filtering/DefaultMavenFileFilter.java:
##########
@@ -19,32 +19,36 @@
* under the License.
*/
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
import java.io.File;
import java.io.IOException;
import java.util.List;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.project.MavenProject;
-import org.apache.maven.shared.utils.io.FileUtils;
-import org.apache.maven.shared.utils.io.FileUtils.FilterWrapper;
-import org.codehaus.plexus.component.annotations.Component;
-import org.codehaus.plexus.component.annotations.Requirement;
import org.sonatype.plexus.build.incremental.BuildContext;
+import static java.util.Objects.requireNonNull;
+
/**
* @author Olivier Lamy
*/
-@Component( role = org.apache.maven.shared.filtering.MavenFileFilter.class, hint = "default" )
+@Singleton
+@Named
public class DefaultMavenFileFilter
extends BaseFilter
implements MavenFileFilter
{
+ private final BuildContext buildContext;
- @Requirement
- private MavenReaderFilter readerFilter;
Review Comment:
Is this one not used anymore?
##########
pom.xml:
##########
@@ -141,21 +136,39 @@
<version>2.2</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-simple</artifactId>
+ <version>${slf4jVersion}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.sonatype.plexus</groupId>
+ <artifactId>plexus-build-api</artifactId>
+ <version>${plexusBuildApiVersion}</version>
+ <scope>test</scope>
+ <classifier>tests</classifier>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.sisu</groupId>
+ <artifactId>org.eclipse.sisu.plexus</artifactId>
+ <version>0.3.0.M1</version><!-- Maven 3.2.5 -->
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.sisu</groupId>
+ <artifactId>org.eclipse.sisu.inject</artifactId>
+ <version>0.3.0.M1</version><!-- Maven 3.2.5 -->
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
<plugins>
<plugin>
- <groupId>org.codehaus.plexus</groupId>
- <artifactId>plexus-component-metadata</artifactId>
- <executions>
- <execution>
- <goals>
- <goal>generate-metadata</goal>
- </goals>
- </execution>
- </executions>
+ <groupId>org.eclipse.sisu</groupId>
+ <artifactId>sisu-maven-plugin</artifactId>
Review Comment:
Are you certain that this isn't in the parent already?
##########
pom.xml:
##########
@@ -141,21 +136,39 @@
<version>2.2</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-simple</artifactId>
+ <version>${slf4jVersion}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.sonatype.plexus</groupId>
+ <artifactId>plexus-build-api</artifactId>
+ <version>${plexusBuildApiVersion}</version>
+ <scope>test</scope>
+ <classifier>tests</classifier>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.sisu</groupId>
+ <artifactId>org.eclipse.sisu.plexus</artifactId>
+ <version>0.3.0.M1</version><!-- Maven 3.2.5 -->
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.eclipse.sisu</groupId>
+ <artifactId>org.eclipse.sisu.inject</artifactId>
+ <version>0.3.0.M1</version><!-- Maven 3.2.5 -->
Review Comment:
Move this to a property please.
##########
src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java:
##########
@@ -90,23 +87,25 @@ public boolean filteredFileExtension( String fileName, List<String> userNonFilte
{
nonFilteredFileExtensions.addAll( userNonFilteredFileExtensions );
}
- String extension = StringUtils.lowerCase( FilenameUtils.getExtension( fileName ) );
+ String extension = getExtension( fileName );
boolean filteredFileExtension = !nonFilteredFileExtensions.contains( extension );
- if ( getLogger().isDebugEnabled() )
+ if ( LOGGER.isDebugEnabled() )
{
- getLogger().debug( "file " + fileName + " has a" + ( filteredFileExtension ? " " : " non " )
+ LOGGER.debug( "file " + fileName + " has a" + ( filteredFileExtension ? " " : " non " )
+ "filtered file extension" );
}
return filteredFileExtension;
}
+ private static String getExtension( String fileName )
+ {
+ String rawExt = FilenameUtils.getExtension( fileName );
+ return rawExt == null ? null : rawExt.toLowerCase( Locale.ROOT );
+ }
+
@Override
public List<String> getDefaultNonFilteredFileExtensions()
{
- if ( this.defaultNonFilteredFileExtensions == null )
- {
- this.defaultNonFilteredFileExtensions = new ArrayList<>();
- }
return this.defaultNonFilteredFileExtensions;
Review Comment:
This changes semantics, 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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org