ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [ant] bodewig commented on pull request #160: Simplify conditions that always produces the same result.
Date Sun, 26 Sep 2021 10:37:24 GMT

bodewig commented on pull request #160:
URL: https://github.com/apache/ant/pull/160#issuecomment-927281071


   Thank you @arturobernalg. I must admit at least I am not a friend of a pull request that
touches 41 unrelated files.
   
   Some of your changes I'm not really happy with. Broadly
   
   * I don't see any harm with a redundant null check.
   * in your change to DirectoryScanner your change decreases readability for me. Where I
could see we are passing on the fast parameter as a method argument I now see a constant and
I need to figure out it is true/false because `fast` is. There are similry changes later.
   * in your change to Tar I don't even see why you are sure the size can never be bigger
than TarConstants.MAX_SIZE :-) (and why the replacement r.size() statement is there)
   * the change to AncestorAnalyzer and the other bcel classes change behavior as we'd now
throw on IOExceptions - see the code comment as for why the code is what it is
   * I don't see why you believe ZipEntry getName() can never return null.
   
   Other team members may feel different about the first two points. This is not a "failed
review", please consider it a remark.


-- 
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: dev-unsubscribe@ant.apache.org

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



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


Mime
View raw message