Skip to main content

Notice: this Wiki will be going read only early in 2024 and edits will no longer be possible. Please see: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/wikis/Wiki-shutdown-plan for the plan.

Jump to: navigation, search

Difference between revisions of "SMILA/Discussions/Checkstyle"

(SMILA Checkstyle configuration changes)
(SMILA Checkstyle configuration changes)
Line 54: Line 54:
 
! Andreas Schank
 
! Andreas Schank
 
! Jürgen Schumacher
 
! Jürgen Schumacher
 +
! Igor Novakovic
 
|-
 
|-
 
| a)
 
| a)
 
| +1  
 
| +1  
 
| +1  
 
| +1  
 +
| -1
 
| -1
 
| -1
 
| -1
 
| -1
Line 66: Line 68:
 
| -1
 
| -1
 
| -1  
 
| -1  
 +
| -1
 
|-
 
|-
 
| c)
 
| c)
Line 72: Line 75:
 
| 0  
 
| 0  
 
| -1
 
| -1
 +
| 0
 
|-
 
|-
 
| d)
 
| d)
 
| +1
 
| +1
 
| +1  
 
| +1  
 +
| +1
 
| +1
 
| +1
 
| +1
 
| +1
 
|-
 
|-
 
| e)
 
| e)
 +
| +1
 
| +1
 
| +1
 
| +1
 
| +1
Line 88: Line 94:
 
| +1
 
| +1
 
| ++1 ;)  
 
| ++1 ;)  
 +
| +1
 
| +1
 
| +1
 
| +1
 
| +1
 
|-
 
|-
 
| t)
 
| t)
 +
| +1
 
| +1
 
| +1
 
| +1
 
| +1
Line 102: Line 110:
 
| +1
 
| +1
 
| +1
 
| +1
 +
| 0
 
|-
 
|-
 
| t2)
 
| t2)
Line 108: Line 117:
 
| 0
 
| 0
 
| +1
 
| +1
 +
| 0
 
|-
 
|-
 
| t3)
 
| t3)
Line 114: Line 124:
 
| +1
 
| +1
 
| +1
 
| +1
 +
| 0
 
|-
 
|-
 
| t4)
 
| t4)
Line 120: Line 131:
 
| 0  
 
| 0  
 
| +1
 
| +1
 +
| 0
 
|-
 
|-
 
| t5)
 
| t5)
Line 126: Line 138:
 
| 0
 
| 0
 
| +1
 
| +1
 +
| 0
 
|-
 
|-
 
| t6)
 
| t6)
 
| -1
 
| -1
 
| 0
 
| 0
 +
| -1
 
| -1
 
| -1
 
| -1
 
| -1
Line 136: Line 150:
 
| +1
 
| +1
 
| +1
 
| +1
 +
| 0
 
| 0
 
| 0
 
| 0
 
| 0
 
|}
 
|}

Revision as of 09:32, 10 March 2011

SMILA Checkstyle configuration changes

I'd like to suggest some changes for the checkstyle settings of our current checkstyle configuration file (brox_checks.xml) here. IMHO, the current configuration is too strict, produces much javadoc overhead and reduces developer acceptance.

I tried to restrict my suggestions to those that should be possible, according to the Checkstyle 5.3 documentation.

I'll label the suggestions with a,b,c,etc. so we can make a voting in the table below. Feel free to add some more suggestions.

Furthermore, I'd suggest a different checkstyle configuration for test bundles with some additional changes, see t1,t2,... below.


Suggestions:

a) exclude "private" methods from javadoc check.

(JS) IMHO it would be sufficient to not require to document every parameter, exception or return value explicitly. We could change this for all kind of methods: A good descriptive text plus good variable names should usually be sufficient as method documentation.

b) exclude "private" variables from javadoc check.

c) remove check for allowed method length (IMHO the methold length is just a hint for too much complexity in a method, but not a real proof - so this should be decided by the developer)

(JS) there are some other "complexity" metrics in checkstyle, maybe we can use these (additionally). On the other hand, the method length *IS* a useful metric: If a method gets too long, it will be hard to understand, because you are not able to get an overview about it. Next, such method tend to need (and have) a lot of inline comments, which make them even longer (Instead of commenting a part of a long method it's better to extract a method - the method name is a comment by itself!). So I think this warning *IS* valid and the developer should think about it. However, for test bundles this limit can be removed.

d) javadoc methods: allow documented exceptions that are not declared if they are a subclass of java.lang.RuntimeException.

e) javadoc methods: allow documented exceptions that are subclass of one of declared exception.

f) allow inline conditionals

t) different checkstyle configuration for test bundles

t1) remove check for "magic numbers".

t2) javadoc methods: allow missing param tags.

t3) javadoc methods: allow missing throws tag.

t4) javadoc methods: allow missing return tag.

t5) remove javadoc check for variables

t6) have no checksstyle for test classes at all.
TM: maybe a bit harsh but i think it still would be OK.
AW: I thought about that too, but finally I'd say that tests are too important to completely get rid of checks.

t7) allow '_' in method and class names (i use this to separate and group the test cases and make them more visible/readable, e.g. Solr_SearchPipelet_ScenarioA_Test, test_feature_variantA_OK, test_feature_variantA_Fail, ... )

Votings:

Andreas Weber Thomas Menzel Andreas Schank Jürgen Schumacher Igor Novakovic
a) +1 +1 -1 -1 -1
b) +1 +1 -1 -1 -1
c) +1 +1 0 -1 0
d) +1 +1 +1 +1 +1
e) +1 +1 +1 +1 +1
f) +1 ++1 ;) +1 +1 +1
t) +1 +1 +1 +1 +1
t1) +1 +1 +1 +1 0
t2) +1 +1 0 +1 0
t3) +1 +1 +1 +1 0
t4) +1 +1 0 +1 0
t5) +1 +1 0 +1 0
t6) -1 0 -1 -1 -1
t7) +1 +1 0 0 0

Copyright © Eclipse Foundation, Inc. All Rights Reserved.