Frank W. Zammetti wrote:
Ted Husted wrote:
You might at least want to start those discussions about the
checkstyle settings, so that we can develop a strategy about how they
would be fixed the next time there is a window of opportunity.
Fair enough...
I saw two issues that seem to account for a large number of the
complaints...
* The issue "Variable xxxx must be private and have accessor methods.".
This is the VisibilityModifierCheck check. I don't think directly
addressing it, i.e., following the suggestion it gives, is the right
answer... I can't imagine it wouldn't break things, either in the SAF
codebase itself or in users' application code.
Fortunately, there appears to be a setting in Checkstyle to deal with this:
http://checkstyle.sourceforge.net/config_design.html#VisibilityModifier
Or we could remove this check entirely. I'd vote for just setting
protectedAllowed to true though.
Since this is a change to the checkstyle rules, not a change to the
codebase, I don't think it needs to wait for a release/GA/whatever.
Might as well just get it done now...
Looking at core alone, that would probably get rid of half of the
remaining issues. This is the only proposed rule change I have at this
point.
* The issue "Expected @throws tag for xxxx". This is coming up a lot
because there are runtime exceptions thrown in methods that are not
declared (which is of course valid, but doesn't follow the style rules).
I would suggestion simply declaring them... I don't *think* that has
any side-effects... anyone think there is?
Declaring runtime exceptions in the throws clause of a method signature
has no impact on the signature, so it should be safe. However, it's
generally considered poor style (on what basis I don't know, doesn't
seem like a problem to me, but...). If there's a checkstyle rule change
that could be used to take care of this then, again, there'd be no need
to wait. If it really does need code changes, it'd be worth holding off
but opening a JIRA issue so we don't forget to come back later.
L.
When I did a a time study in February, curing the the current errors
with the current settings would take at least 40 hard hours. We need
to find a way to do the work faster, or get more people to work on it.
Agreed, and your estimate may even be too optimistic given that first
bullet above, if the rule change wasn't implemented. I frankly don't
see another way to deal with that particular complaint.
(We probably should have made this a Google Summer of Code project.)
Hehe, yeah :)
There is also the issue of how we want to handle the exception issues.
We will be addressing exception handling in SAF 2, so it's a pertinent
question.
I certainly don't disagree that some exception handling updates should
be looked at, but I'm not sure that's pertinent in the context of
Checkstyle complaints... the exception bullet above I don't think should
be expanded to modifying how exceptions are handled now, it's just
getting rid of the complaints, which I think is as simple as declaring
the exceptions.
We might also consider fixing some of the errors at a time, for
example maybe just the exception handlers. That could have less of an
impact that trying to cure all six thousand at once.
Same point as the above paragraph... I think we're talking about two
different things really.
Although, if you wanted to tackle just one category of complaint at a
time, that would be fine... I was thinking of a package at a time, but
the work breakdown can certainly be done any of a number of ways.
Ultimately though, that one rule change above would cut down a pretty
substantial number of complaints, and in this case I don't think
changing the rules is a cop-out. So, I'd like to see a consensus
reached on that alone, everything else could be back-burnered as far as
I'm concerned.
-Ted.
Frank
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
