bodewig commented on a change in pull request #115:
URL: https://github.com/apache/ant/pull/115#discussion_r462222659



##########
File path: src/main/org/apache/tools/ant/listener/Log4jListener.java
##########
@@ -142,21 +142,17 @@ public void messageLogged(final BuildEvent event) {
         final Logger log
             = Logger.getLogger(categoryObject.getClass().getName());
         switch (event.getPriority()) {
-            case Project.MSG_ERR:
-                log.error(event.getMessage());
-                break;
             case Project.MSG_WARN:
                 log.warn(event.getMessage());
                 break;
             case Project.MSG_INFO:
                 log.info(event.getMessage());
                 break;
             case Project.MSG_VERBOSE:
-                log.debug(event.getMessage());
-                break;
             case Project.MSG_DEBUG:
                 log.debug(event.getMessage());
                 break;
+            case Project.MSG_ERR:

Review comment:
       not explicit enough for my taste :-)

##########
File path: src/main/org/apache/tools/ant/taskdefs/MacroInstance.java
##########
@@ -221,8 +221,6 @@ private String macroSubs(String s, Map<String, String> 
macroMapping) {
             }
         }
         switch (state) {
-            case STATE_NORMAL:
-                break;

Review comment:
       doesn't make me like it much more

##########
File path: 
src/main/org/apache/tools/ant/taskdefs/optional/jlink/ClassNameReader.java
##########
@@ -86,14 +87,9 @@
                 values[i] = data.readUnsignedShort();
                 break;
 
-            case FIELDREF :
-            case METHODREF :
-            case INTERFACEMETHODREF :
-            case NAMEANDTYPE :
-                values[i] = data.readInt();
-                break;
+            case UNUSED :

Review comment:
       let me note a pattern here, Personally I do not like cases that fall 
through to the `default` case.

##########
File path: src/main/org/apache/tools/ant/taskdefs/optional/jsp/WLJspc.java
##########
@@ -178,7 +178,7 @@ public void execute() throws BuildException {
                 args[j + 1] = destinationPackage;
             } else {
                 parents = this.replaceString(parents, File.separator, "_.");
-                args[j + 1] = destinationPackage + "." + "_" + parents;
+                args[j + 1] = destinationPackage + "._" + parents;

Review comment:
       please avoid mixing unrelated commits with your PRs.

##########
File path: src/main/org/apache/tools/ant/taskdefs/optional/net/FTP.java
##########
@@ -2690,6 +2680,8 @@ public int getAction() {
                 return RM_DIR;
             case "site":
                 return SITE_CMD;
+            case "send":
+            case "put":

Review comment:
       see above ;-)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Reply via email to