You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "baneja (via GitHub)" <gi...@apache.org> on 2023/04/07 01:30:51 UTC
[GitHub] [maven-assembly-plugin] baneja opened a new pull request, #131: Refactored code for Maven Assembly Plugin. No code breaks, all test cases passing.
baneja opened a new pull request, #131:
URL: https://github.com/apache/maven-assembly-plugin/pull/131
### I have refactored the code using following techniques in the mentioned classes:
1. Refactoring name: **Extract Method**
Location: src/main/java/org/apache/maven/plugins/assembly/utils/AssemblyFileUtils.java
Line 69
2. Refactoring name: **Introduce explaining variable**
Location: src/main/java/org/apache/maven/plugins/assembly/mojos/SingleAssemblyMojo.java
Line 71
3. Refactoring name: **Rename Method**
Location: src/main/java/org/apache/maven/plugins/assembly/format/ReaderFormatter.java
Line 95
4. Refactoring name: **Replace conditional with Polymorphism**
Location: src/main/java/org/apache/maven/plugins/assembly/io/DefaultMessageHolder.java
Line 514
5. 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
6. Refactoring name: **Push Down Method**
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
--
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-assembly-plugin] slawekjaranowski commented on a diff in pull request #131: Refactored code for Maven Assembly Plugin. No code breaks, all test cases passing.
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
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
[GitHub] [maven-assembly-plugin] elharo closed pull request #131: Refactored code for Maven Assembly Plugin. No code breaks, all test cases passing.
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo closed pull request #131: Refactored code for Maven Assembly Plugin. No code breaks, all test cases passing.
URL: https://github.com/apache/maven-assembly-plugin/pull/131
--
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-assembly-plugin] elharo commented on pull request #131: Refactored code for Maven Assembly Plugin. No code breaks, all test cases passing.
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on PR #131:
URL: https://github.com/apache/maven-assembly-plugin/pull/131#issuecomment-1623552059
probable bot spam
--
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