On 24/02/2013 22:43, Konstantin Kolinko wrote: > 2013/2/24 <ma...@apache.org>: >> Author: markt >> Date: Sun Feb 24 19:56:33 2013 >> New Revision: 1449540 >> >> URL: http://svn.apache.org/r1449540 >> Log: >> Checkstyle: remove nested blocks >> (Testing git-svn setup on new machine) > > -1. > From a quick look at it: > 1) Nothing wrong with those blocks in switches. I think removing them > reduces readability. Blocks are useful when you need a local variable > there. If just some cases have them it would be inconsistent.
They are unnecessary. The blocks don't add anything in terms of readability that the indentation doesn't already provide. Not all cases have blocks - they are already inconsistent. Most case statements don't use blocks so this change actually improves things. > 2) Comments in ClusterRuleSet have their scope. That scope limitation > is lost when you remove blocks. The comment itself is probably unnecessary. The name of the prefix - channelPrefix makes it pretty clear what those properties are for. I left the comment as it does provide a clear visual clue as to where the properties start. > 3) Variables in SessionUtils that were local to some logical block are > now having wider scope, for no good reason. And no particular harm either. Removing the blocks in DOMWriter identified a small amount of duplicate code that was also removed. I view that as a good thing. Overall I'm not particularly bothered if this commit is reverted if you feel strongly about it. My main motivation was to test git-svn. Fixing some issues highlighted by a currently commented out checkstyle test seemed like a good thing to test it with. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
