hama-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Jungblut <thomas.jungb...@googlemail.com>
Subject Re: Code Quality
Date Thu, 14 Jun 2012 15:25:34 GMT
Yea, it would be great if we can find some automatism, maybe in maven like
RAT that is failing the build if too many warnings or errors have been
added. Does Sonar provide something equal or is is something plugged into
jenkins (would be some improvement anyways)?
Documentation of generics or general API is very sparse, you're right. We
have to improve on this one as well.

+1 for the code review.

2012/6/14 Tommaso Teofili <tommaso.teofili@gmail.com>

> 2012/6/14 Tommaso Teofili <tommaso.teofili@gmail.com>
>
> > Hi Thomas,
> >
> > big +1 to your comments, I also encountered some of the things you
> > mentioned, that's why I opened .
> >
>
> missed to paste the link to https://issues.apache.org/jira/browse/HAMA-572
>
>
> > I think that we should now release 0.5.0 and then plan for an appropriate
> > API / code review.
> > Just for the sake of an example, the BSP interface (a very important one)
> > has generics which don't have neither binding to interfaces nor
> > documentation to explain what K, V, etc. mean.
> >
> > We may want to use the ASF Sonar instance to enable a semi automatic way
> > of checking for this kind of bugs.
> > My 2 cents,
> > Tommaso
> >
> > 2012/6/14 Thomas Jungblut <thomas.jungblut@googlemail.com>
> >
> >> Hi all,
> >>
> >> I'm currently going through all packages and cleaning up code by eclipse
> >> warnings. There I found some huge bugs, for example:
> >>
> >> public final void add(List<Metric<? extends Number>> metrics){
> >> >   metrics.addAll(metrics);
> >> > }
> >>
> >>
> >> Which is basically not intended, but wanted to reference a field name
> >> "metrics". This part is just adding the same collection to itself.
> >> Also I have seen many not using override annotations or using generics.
> >> There are also many open resources that are not properly closed with
> >> finally, thus possible descriptor leaks.
> >> There are several places, also I did some faults because several
> warnings
> >> are not turned on in the latest eclipse version.
> >>
> >> I would advise you (at least the comitters) to turn on the code
> >> inspections
> >> in your IDE.
> >> I have turned on several things in eclipse: (can be found in the project
> >> preferences or global preferences under Java/Compiler/"Errors/Warnings")
> >>
> >> From top to bottom:
> >>
> >> Non-static access to static member (WARNING)
> >> Indirect access to static member (WARNING)
> >> Method with a constructor name (WARNING)
> >> Parameter assignment (WARNING)
> >> Method can be static (WARNING)
> >>
> >> Serializable class without serialID (WARNING)
> >> Assignment has no effect (WARNING)
> >> finally does not complete normally (WARNING)
> >> Using a char array in string concat (WARNING)
> >> hidden catch block (WARNING)
> >> Inexact type match for varags argument (WARNING)
> >> Nullpointer access (WARNING)
> >> Compare identical values (WARNING)
> >> class overrides equals but not hashcode (WARNING)
> >> dead code (WARNING)
> >>
> >> Type parameter hides another type (WARNING)
> >> Method does not override package visible method (WARNING)
> >> interface method conflicts with protected object method (WARNING)
> >>
> >> Deprecated API (WARNING) // can be neglected by using annotations if not
> >> other possible though
> >> Forbidden references (ERROR)
> >> Discouraged references (WARNING)
> >>
> >> Value of local variable is not used (WARNING) // please delete it if
> never
> >> read
> >> Unused import (WARNING) // please format and organize imports before
> >> making
> >> a patch (in eclipse CTRL+SHIFT+F and CTRL+O)
> >> Unused private member (WARNING) // remove if never used
> >> Unnecessary cast or instanceof operation (WARNING)
> >> Unused break or continue label (WARNING)
> >>
> >> Unchecked generic type operation (WARNING)
> >> Usage of raw types (WARNING) // sometimes can not be avoided, can be
> >> neglected via annotation
> >> Generic type parameter declared with a final type bound (WARNING)
> >>
> >> Missing override annotation (WARNING)
> >> Annotation is used as super interface (WARNING)
> >> Unhandled token in SuppressWarnings (WARNING)
> >>
> >> I will put this up in our wiki and I want that everybody who commits a
> >> patch is checking against these guidelines, I don't want to add Findbugs
> >> because the false positive rate is too high.
> >> Would be great if everybody will care about that, because static code
> >> analysis is very great and if it's offered we should use it.
> >>
> >> --
> >> Thomas Jungblut
> >> Berlin <thomas.jungblut@gmail.com>
> >>
> >
> >
>



-- 
Thomas Jungblut
Berlin <thomas.jungblut@gmail.com>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message