You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "slawekjaranowski (via GitHub)" <gi...@apache.org> on 2023/04/26 16:45:23 UTC
[GitHub] [maven-assembly-plugin] slawekjaranowski commented on a diff in pull request #131: Refactored code for Maven Assembly Plugin. No code breaks, all test cases passing.
slawekjaranowski commented on code in PR #131:
URL: https://github.com/apache/maven-assembly-plugin/pull/131#discussion_r1178127866
##########
src/main/java/org/apache/maven/plugins/assembly/archive/archiver/PrefixedFileSet.java:
##########
@@ -45,32 +42,9 @@ class PrefixedFileSet implements FileSet {
PrefixedFileSet(final FileSet fileSet, final String rootPrefix, final FileSelector[] selectors) {
this.fileSet = fileSet;
this.selectors = selectors;
-
- if (rootPrefix.length() > 0 && !rootPrefix.endsWith("/")) {
- this.rootPrefix = rootPrefix + "/";
- } else {
- this.rootPrefix = rootPrefix;
- }
- }
-
- /**
- * {@inheritDoc}
- */
- static FileSelector[] combineSelectors(FileSelector[] first, FileSelector[] second) {
- if ((first != null) && (second != null)) {
- final FileSelector[] temp = new FileSelector[first.length + second.length];
-
- System.arraycopy(first, 0, temp, 0, first.length);
- System.arraycopy(second, 0, temp, first.length, second.length);
-
- first = temp;
- } else if ((first == null) && (second != null)) {
- first = second;
- }
-
- return first;
+ // Refactored using extract class. created new class RootPrefix
Review Comment:
comment not needed
##########
src/main/java/org/apache/maven/plugins/assembly/archive/archiver/PrefixedFileSet.java:
##########
@@ -159,4 +123,19 @@ public InputStreamTransformer getStreamTransformer() {
public FileMapper[] getFileMappers() {
return EMPTY_FILE_MAPPERS_ARRAY;
}
+
+ static FileSelector[] combineSelectors(FileSelector[] first, FileSelector[] second) {
Review Comment:
looks not changed - please don't change position
##########
src/main/java/org/apache/maven/plugins/assembly/archive/archiver/PrefixedFileSet.java:
##########
@@ -106,16 +78,8 @@ public String getPrefix() {
if (prefix == null) {
return rootPrefix;
}
-
- if (prefix.startsWith("/")) {
- if (prefix.length() > 1) {
- prefix = prefix.substring(1);
- } else {
- prefix = "";
- }
- }
-
- return rootPrefix + prefix;
+ // Refactored using extract class. created new class PrefixedPrefix
+ return new PrefixedPrefix(rootPrefix, prefix).getValue();
Review Comment:
code used in one place - I don't see benefit to extract
##########
src/main/java/org/apache/maven/plugins/assembly/archive/archiver/PrefixedFileSet.java:
##########
@@ -45,32 +42,9 @@ class PrefixedFileSet implements FileSet {
PrefixedFileSet(final FileSet fileSet, final String rootPrefix, final FileSelector[] selectors) {
this.fileSet = fileSet;
this.selectors = selectors;
-
- if (rootPrefix.length() > 0 && !rootPrefix.endsWith("/")) {
- this.rootPrefix = rootPrefix + "/";
- } else {
- this.rootPrefix = rootPrefix;
- }
- }
-
- /**
- * {@inheritDoc}
- */
- static FileSelector[] combineSelectors(FileSelector[] first, FileSelector[] second) {
- if ((first != null) && (second != null)) {
- final FileSelector[] temp = new FileSelector[first.length + second.length];
-
- System.arraycopy(first, 0, temp, 0, first.length);
- System.arraycopy(second, 0, temp, first.length, second.length);
-
- first = temp;
- } else if ((first == null) && (second != null)) {
- first = second;
- }
-
- return first;
+ // Refactored using extract class. created new class RootPrefix
+ this.rootPrefix = new RootPrefix(rootPrefix).getValue();
Review Comment:
code used in one place - I don't see benefit to extract
##########
src/main/java/org/apache/maven/plugins/assembly/format/ReaderFormatter.java:
##########
@@ -92,7 +92,7 @@ private static boolean isForbiddenFiletypes(PlexusIoResource plexusIoResource) {
return (fileName.endsWith(".zip") || fileName.endsWith(".jar"));
}
- private static void checkifFileTypeIsAppropriateForLineEndingTransformation(PlexusIoResource plexusIoResource)
+ /* private static void checkifFileTypeIsAppropriateForLineEndingTransformation(PlexusIoResource plexusIoResource)
Review Comment:
We don't need preserve old code as comment
--
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