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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
