In general, doing this sort of nagging on the developer list is *exactly* the right thing to do for getting patches you believe in moved forward :-). I'm not directly involved in Struts 1.3 development so I'll leave overall acceptance to others, but a couple of general comments on your suggestions follow (snipping to focus on just the relevant issues).
On 8/8/05, Frank W. Zammetti <[EMAIL PROTECTED]> wrote: [snip] > * StrictDuplicateCode > Duplicate code is just plain bad... not in terms of something not > working, although that is certainly possible in some situations, but > it's just a sign of carelessness. If nothing else, I'm sure no one > wants the Struts code base to show any carelessness that could easily > be avoided. Although the general principle is sound, there can be a counter-argument that cut-n-paste can sometimes avoid undesireable cross-package dependencies. Most rules engines I've seen allow you to create an exceptions list where the developers say "yes, I know this violates the style rule, but we want it this way anyway." Does CheckStyle have that feature? [snip] > * ImportOrder > Imports in alphabetical order just makes it a little easier to find if > something is used. This is obviously not going to break anything, and > is easy to fix with virtually any IDE in existence today (or an > already-mapped macro in UltraEdit for us anti-IDE folk), so it's > low-hanging fruit, might as well grab it I figure :) I assume we also want to prohibit wild card imports? Besides driving me nuts, they are also semantically dangerous. > * IllegalCatch > Again, as the docs say: "Catching java.lang.Exception, java.lang.Error > or java.lang.RuntimeException is almost never acceptable." In this > cases where it actually *IS* acceptable, simply ignoring the error is > acceptable. "Simply ignoring" doesn't work if you are catching checked exceptions, so again I would hope this could be selectively turned off on individual use cases. [snip] > * UnnecessaryParentheses > I personally find code with parenthesis that aren't actually needed to > be more difficult, some feel the opposite. If they are truly not > needed though, it's just more characters for my brain to parse I > figure. Personally, I've been burnt enough times by assuming developers know what the operator hierarchy is to err on the side of redundant parentheses whenever there is any possible doubt. On the other hand, I could easily be convinced that redundant parentheses around a return value don't add anything useful, so it wouldn't bother me to need to remove those. Without examining the current reports, I'll bet this one triggers a bunch of warnings on the Struts code base. [snip] > The first is with the current rules file, resulting in 4829 complaints > (many of which are relatively minor things, i.e., tabs instead of > spaces, javadoc problems, etc.) We can catch tabs instead of spaces? Cool! :-) > Frank W. Zammetti Craig --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
