You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2014/09/08 00:49:17 UTC
svn commit: r1623243 - /tomcat/trunk/res/checkstyle/checkstyle.xml
Author: markt
Date: Sun Sep 7 22:49:17 2014
New Revision: 1623243
URL: http://svn.apache.org/r1623243
Log:
Enable the nested blocks check
Modified:
tomcat/trunk/res/checkstyle/checkstyle.xml
Modified: tomcat/trunk/res/checkstyle/checkstyle.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/res/checkstyle/checkstyle.xml?rev=1623243&r1=1623242&r2=1623243&view=diff
==============================================================================
--- tomcat/trunk/res/checkstyle/checkstyle.xml (original)
+++ tomcat/trunk/res/checkstyle/checkstyle.xml Sun Sep 7 22:49:17 2014
@@ -43,9 +43,9 @@
value="${tomcat.output}/res/checkstyle/cachefile-checkstyle.xml"/>
<!-- Block Checks -->
- <!-- ~60 errors
- <module name="AvoidNestedBlocks"/>
- -->
+ <module name="AvoidNestedBlocks">
+ <property name="allowInSwitchCase" value="true"/>
+ </module>
<!-- Coding -->
<module name="IllegalInstantiation"/>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1623243 - /tomcat/trunk/res/checkstyle/checkstyle.xml
Posted by Mark Thomas <ma...@apache.org>.
On 09/09/2014 11:06, Konstantin Kolinko wrote:
> 2014-09-08 2:49 GMT+04:00 <ma...@apache.org>:
>> Author: markt
>> Date: Sun Sep 7 22:49:17 2014
>> New Revision: 1623243
>>
>> URL: http://svn.apache.org/r1623243
>> Log:
>> Enable the nested blocks check
>>
>> Modified:
>> tomcat/trunk/res/checkstyle/checkstyle.xml
>>
>> Modified: tomcat/trunk/res/checkstyle/checkstyle.xml
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/res/checkstyle/checkstyle.xml?rev=1623243&r1=1623242&r2=1623243&view=diff
>> ==============================================================================
>> --- tomcat/trunk/res/checkstyle/checkstyle.xml (original)
>> +++ tomcat/trunk/res/checkstyle/checkstyle.xml Sun Sep 7 22:49:17 2014
>> @@ -43,9 +43,9 @@
>> value="${tomcat.output}/res/checkstyle/cachefile-checkstyle.xml"/>
>>
>> <!-- Block Checks -->
>> - <!-- ~60 errors
>> - <module name="AvoidNestedBlocks"/>
>> - -->
>> + <module name="AvoidNestedBlocks">
>> + <property name="allowInSwitchCase" value="true"/>
>> + </module>
>>
>> <!-- Coding -->
>> <module name="IllegalInstantiation"/>
>
> FYI: I do not like this change.
>
> Limiting the scope of some local variables helps to isolate logic
> blocks and helps to avoid some copy-paste errors when you forget to
> rename a helper variable in a copy-pasted block.
There was one case of using that technique in the main code and one in
the test code. I think it is fair to say it hasn't been widely used.
The main thing I have against it is readability as - to my eye at least
- it makes it harder to read.
Cheers,
Mark
>
> E.g.
> http://svn.apache.org/r1623238
> http://svn.apache.org/r1623241
>
>
> There was no technical issues fixed by these changes, so there are not
> much to discuss here. I am just afraid that forcing such a rule will
> lead to coding errors.
>
> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1623243 - /tomcat/trunk/res/checkstyle/checkstyle.xml
Posted by Konstantin Kolinko <kn...@gmail.com>.
2014-09-08 2:49 GMT+04:00 <ma...@apache.org>:
> Author: markt
> Date: Sun Sep 7 22:49:17 2014
> New Revision: 1623243
>
> URL: http://svn.apache.org/r1623243
> Log:
> Enable the nested blocks check
>
> Modified:
> tomcat/trunk/res/checkstyle/checkstyle.xml
>
> Modified: tomcat/trunk/res/checkstyle/checkstyle.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/res/checkstyle/checkstyle.xml?rev=1623243&r1=1623242&r2=1623243&view=diff
> ==============================================================================
> --- tomcat/trunk/res/checkstyle/checkstyle.xml (original)
> +++ tomcat/trunk/res/checkstyle/checkstyle.xml Sun Sep 7 22:49:17 2014
> @@ -43,9 +43,9 @@
> value="${tomcat.output}/res/checkstyle/cachefile-checkstyle.xml"/>
>
> <!-- Block Checks -->
> - <!-- ~60 errors
> - <module name="AvoidNestedBlocks"/>
> - -->
> + <module name="AvoidNestedBlocks">
> + <property name="allowInSwitchCase" value="true"/>
> + </module>
>
> <!-- Coding -->
> <module name="IllegalInstantiation"/>
FYI: I do not like this change.
Limiting the scope of some local variables helps to isolate logic
blocks and helps to avoid some copy-paste errors when you forget to
rename a helper variable in a copy-pasted block.
E.g.
http://svn.apache.org/r1623238
http://svn.apache.org/r1623241
There was no technical issues fixed by these changes, so there are not
much to discuss here. I am just afraid that forcing such a rule will
lead to coding errors.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org