-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactored code for Maven Assembly Plugin. No code breaks, all test cases passing. #131
Conversation
} | ||
|
||
return first; | ||
// Refactored using extract class. created new class RootPrefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment not needed
@@ -159,4 +123,19 @@ public InputStreamTransformer getStreamTransformer() { | |||
public FileMapper[] getFileMappers() { | |||
return EMPTY_FILE_MAPPERS_ARRAY; | |||
} | |||
|
|||
static FileSelector[] combineSelectors(FileSelector[] first, FileSelector[] second) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks not changed - please don't change position
|
||
return rootPrefix + prefix; | ||
// Refactored using extract class. created new class PrefixedPrefix | ||
return new PrefixedPrefix(rootPrefix, prefix).getValue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code used in one place - I don't see benefit to extract
|
||
return first; | ||
// Refactored using extract class. created new class RootPrefix | ||
this.rootPrefix = new RootPrefix(rootPrefix).getValue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code used in one place - I don't see benefit to extract
@@ -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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need preserve old code as comment
probable bot spam |
I have refactored the code using following techniques in the mentioned classes:
Refactoring name: Extract Method
Location: src/main/java/org/apache/maven/plugins/assembly/utils/AssemblyFileUtils.java
Line 69
Refactoring name: Introduce explaining variable
Location: src/main/java/org/apache/maven/plugins/assembly/mojos/SingleAssemblyMojo.java
Line 71
Refactoring name: Rename Method
Location: src/main/java/org/apache/maven/plugins/assembly/format/ReaderFormatter.java
Line 95
Refactoring name: Replace conditional with Polymorphism
Location: src/main/java/org/apache/maven/plugins/assembly/io/DefaultMessageHolder.java
Line 514
Refactoring name: Extract Class
Location: src/main/java/org/apache/maven/plugins/assembly/archive/archiver/PrefixedFileSet.java
Line 46 and 82
Made new classes
RootPrefix.java: src/main/java/org/apache/maven/plugins/assembly/archive/archiver/RootPrefix.java
Prefix Prefixed.java: src/main/java/org/apache/maven/plugins/assembly/archive/archiver/PrefixedPrefix.java
Location: FileLocation.java: src/main/java/org/apache/maven/plugins/assembly/io/FileLocation.java
Line 145
URLLocation.java: src/main/java/org/apache/maven/plugins/assembly/io/URLLocation.java
Line 81