Re: Questions about Development Practices

[Colin McCabe <cm...@apache.org>]

On Sun, Apr 2, 2017, at 13:28, Daan Rennings wrote:
> Dear developers,
> 
> When going over your way of working adopted with Apache Kafka, I was
> wondering about the following:
> 
> *1) Visualizing Technical Debt*
> Based on my findings (with FindBugs, CheckStyle and JaCoCo), I concluded
> that Kafka's codebase has a good overall quality with regard to the
> architecture and individual lines of code (great work!). I was however
> wondering, have you ever considered to also include reports from e.g.
> Sonarqube to make this quality more visuable? I believe it's nice for
> programmers to see that they are delivering a good job (through e.g. the
> Sonarqube panel after fixing some issues with regard to technical debt),
> but technical debt would also become even more manageable through the
> adoption of such a tool (with regard to the latter, one could also think
> of
> e.g. using CodeCity).

Hi Daan,

That's interesting.  Jenkins provides some of the functionality you
describe, in terms of giving developers a dashboard to look at.  I think
the biggest improvement we could make is probably improving test
stability there.  There have also been proposals to add a test coverage
checking tool to our Jenkins pre-commit build, which would be a welcome
improvement for anyone who wants to contribute.

> 
> *2) Adaptations from FindBugs and CheckStyle's defaults*
> Based on the findbugs-exclude.xml and checkstyle.xml, I found that you
> have
> decided to deviate from some default values (e.g. exluding bugs with
> regard
> to MS (Malicious code vulnerabilities) and NPathComplexity of max 500
> instead of the default 200). Is there any documentation on the decision
> made for these deviations? Or, if not, could you elaborate upon your
> choices?

In general, we found that the findbugs MS warnings were not practical to
fix.  For example, it would have meant getting rid of all public array
fields, as well as any function that returned a mutable reference to
anything inside a class.  This would have required a rewrite of many
internal classes, for unclear benefit.  Kafka was also originally a
Scala project, so there is a philosophy of sometimes leaving internal
data fields public or package-private, as is typically done in Scala. 
We have been moving away from this in the Java code, and towards putting
things behind accessors, but a lot of code still eschews them.  These
issues tend to be more important when you are writing library APIs than
in internal code.

With regard to NPathComplexity, I think that the reasoning was that
setting it to 200 was causing it to fail too much existing code.  I'm
actually not a big fan of this checkstyle rule in the first place, since
I think the complexity of code is better assessed by a human than by
checkstyle.  I do think the naming and whitespace rules enforced by
checkstyle are useful.

cheers,
Colin

> 
> Thank you very much in advance!
> 
> Kind regards,
> 
> Daan Rennings
> 
> P.S. I am with a team of students from Delft University of Technology,
> trying to analyze Apache Kafka as part of the course "IN4315 Software
> Architecture" which will publish it's findings in a GitBook (for more
> information, please have a look at
> https://avandeursen.com/2017/01/15/the-collaborative-software-architecture-course/).
> Answers to both questions would be useful for our analysis of Apache
> Kafka.