On 12 Sep 2014 at 18:46:06, Sergiu Dumitriu
([email protected](mailto:[email protected])) wrote:
> A tool still being maintained and released isn't dead.
Actually you’re right. It has been unmaintained for a long time but apparently
last year 2 committers picked it up and moved it to GitHub to start a new life
for it.
Thanks
-Vincent
> The fact that
> there's healthy competition is good, not bad, for a project. And we're
> not bound to using the most complex tool available, we should use the
> one which works best for us, where "best" should be agreed on by the
> community.
>
> There should be a separate vote about switching from Checkstyle to
> Sonar. If you send such a vote, we can put this on hold until we decide
> one way or the other.
>
> It's kind of backwards to agree on the rules before agreeing on the tool
> first.
>
> On 09/12/2014 12:38 PM, [email protected] wrote:
> > Hi,
> >
> > I haven’t had time to read this thread yet. Very generally:
> > - checkstyle is a dead tool
> > - the sonar guys have been reimplementing the rules in a better way
> > - sonar offers more rules (pmd, findbugs, etc)
> > - I’ve started defining rules we may want to use in this email thread:
> > http://xwiki.markmail.org/thread/vnggoomrbqbfaojn
> > - I’ve already excluded rules, listed on
> > http://design.xwiki.org/xwiki/bin/view/Proposal/DefineSonarRules
> > - One next step is to look at http://sonar.xwiki.org to see the remaining
> > violations and decide which one we want to remove
> > - Another step will be to decide to use the sonar maven plugin to act as a
> > replacement for the checkstyle maven plugin and fail the build in case of
> > errors
> > - The list of currently enabled rules are available here:
> > http://sonar.xwiki.org/coding_rules#qprofile=java-xwiki-57409|activation=true|languages=java|s=createdAt|asc=false
> >
> > Thanks
> > -Vincent
> >
> > On 12 Sep 2014 at 18:27:50, Sergiu Dumitriu
> > ([email protected](mailto:[email protected])) wrote:
> >
> >> I've been working on and off on updating our usage of Checkstyle, since
> >> we haven't really updated our checkstyle configuration since we first
> >> started using it, other than minor tweaks. In the latest version of the
> >> Checkstyle tool several useful checks have been added, which I'd like to
> >> enable.
> >>
> >> On 09/12/2014 04:15 AM, Caleb James DeLisle wrote:
> >>> Hi all,
> >>>
> >>> This is partially an extension of the previous mail thread
> >>> ``Checkstyle audit audit'' where in the discussion, I formed a few
> >>> thoughts
> >>> about checkstyle rules which should be added and removed. Since that
> >>> thread
> >>> is very cluttered now, I wanted to begin a new one specifically explaining
> >>> the changes which I think we should make. Comments are requested.
> >>>
> >>>
> >>> All but the most trivial Codestyle rules all create friction which slows
> >>> down
> >>> work. They are however valuable because they identify likely mistakes and
> >>> make
> >>> it easier for others to read one's code. Still we must periodically
> >>> review our
> >>> rules and identify both new rules which can help us and existing rules
> >>> which
> >>> are more bother than they are help, either in general or in specific to
> >>> the
> >>> experience level of our team.
> >>>
> >>>
> >>> Changes I would like to make:
> >>>
> >>> #1: Remove "duplicate strings" checkstyle rule. This rule is supposed to
> >>> trap
> >>> people who are hardcoding constants throughout their code instead of
> >>> defining
> >>> them in one place. However when writing this email, I tried to find a
> >>> coherent
> >>> example which is much improved by hoisting the string value up into a
> >>> constant
> >>> and I cannot think of a single example. False positive examples spring to
> >>> mind
> >>> immediately, usually log/error messages or on strings which are sure
> >>> never to
> >>> change (at least not before the next major refactoring). "fixing" these
> >>> errors
> >>> makes the code worse because one can no-longer read top to bottom, one
> >>> must
> >>> scroll up and down between constants and code. I think this rule is more
> >>> harm
> >>> than good, especially for our team's experience level.
> >>
> >> +0 on this one, it's an extra safety net, but one that I agree is a bit
> >> too cumbersome, and I'm not a big fan of defining private constants just
> >> to work around this rule.
> >>
> >>> #2: Set class/method/field javadoc requirement to exempt private and
> >>> package
> >>> private entities. When I write code, I want to do everything the easiest
> >>> way
> >>> that could work. When I read code, I want every comment to be giving me
> >>> important and up-to-date information. If checkstyle makes me write
> >>> comments
> >>> which teach programming, I'm going to be mad at checkstyle, if I try to
> >>> debug
> >>> your code and the important details/warnings are mixed in with a rash of
> >>> CS-101
> >>> babble, I'm going to be mad at you. In either case it takes me longer to
> >>> do
> >>> what I set out to do.
> >>> The solution of requiring javadoc only on public classes/interfaces also
> >>> helps
> >>> me when developing because I need to think more deeply about the
> >>> interfaces
> >>> which I am exposing to the world. "write javadoc here" or
> >>> "this needs an @param tag" implies
> >>> "hey, did you really intend to make that public API?".
> >>> I think the change not only lifts the burden on the programmer but also
> >>> actually helps checkstyle achieve it's goals more efficiently.
> >>
> >> +1. All public code should be thoroughly documented, and private code
> >> should only document what's not obvious.
> >>
> >>> #3: Add rule requiring use of 'this.' when accessing class fields and
> >>> remove
> >>> rule preventing variable names which shadow fields. This change makes
> >>> checkstyle
> >>> *more* strict but I believe the cost is reasonable and the benefit is
> >>> high.
> >>> Reading your code I immediately know whether you're accessing a field or a
> >>> variable which tells me whether there is reason why your function might
> >>> behave
> >>> differently depending on the state of the object. In light of this change,
> >>> there is nolonger any reason to concern ourselves with shadowing of field
> >>> names since we cannot access them this way so I think the new rule should
> >>> be accompanied by the removal of the shadowing rule.
> >>
> >> +1, but there's currently a bug in that rule:
> >> https://github.com/checkstyle/checkstyle/pull/274
> >>
> >> More rules to enable in https://github.com/xwiki/xwiki-commons/pull/9
>
> --
> Sergiu Dumitriu
> http://purl.org/net/sergiu
> _______________________________________________
> devs mailing list
> [email protected]
> http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
