On 1/20/06, Niall Pemberton <[EMAIL PROTECTED]> wrote:
> I haven't time to look through all 50 commit messages for this change - but
> a quick look at a few of the messages already brought up a couple of issues:
>
> * The reformatting of the svn keyswords @version $Rev$ $Date$ has moved the
> $ sign from the end of Date to the next line - effectively disabling the
> keyword and committing the expanded text - saw this on FormBeanConfig and
> FormPropertyConfig - assume its happened everywhere.

When the macro expands, it can exceed 80 characters per line, which
conflicts with the CheckStyle settings. To use the SVN macro, we
should increase the standard line length to at least 85 characters.
When this setting is changed, the line is automattically rewrapped, so
that it looks like this

* @version $Rev: 371073 $ $Date: 2005-08-26 21:58:39 -0400 (Fri, 26 Aug 2005) $


 > * The class javadoc for ActionServlet - has merged all the list items (i.e.
> <li>....</li>) into one big paragraph - same things happended in
> FormPropertyConfig on the javadoc for the initial() method. I guess anywhere
> we've used lists in the javadoc, a similar thing has happened.

A related problem is that some tags that we have used, like <code>,
are being excused from the line count by IDEA, but counted by
Checkstyle. As a result, lines that use <code> may exceed 80
characters, even after automatic reformatting, and continue to
generate Checkstyle errors.

As to the markup for the list, one fix is to put blank lines around
the list, like this:

 *
 * <ul>
 *
 * <li>Instance and static variables MUST NOT be used to store
information related to
 * the state of a particular request. They MAY be used to share global resources
 * across requests for the same action.</li>
 *
 * <li>Access to other resources (JavaBeans, session variables, etc.) MUST be
 * synchronized if those resources require protection. (Generally,
however, resource
 * classes should be designed to provide their own protection where
necessary.</li>
 *
 * </ul>
 *

Which, I'd be happy to do.

> Alot of the changes seem trivial and unnecessary - much of the reformatting
> IMO doesn't improve anything. Where it has made major changes is to
> re-organise the order of variables/methods which is going to make it more
> difficult to look back and find out when/if something changed. The other
> thing is that this is such a big change there is no way to check it all out
> and be sure that it hasn't screwed up in other ways, other than just
> formatting. Before seeing the effects of automated re-formatting, I was
> neutral - now I'm pretty sure I don't like it. Something passive like
> checkstyle is fine, but this direction seems like bad news to me.

I think the truly bad news is that, throughout the project, we have
more than 23,395 checkstyle errors :(

Yes, 23,395+

Of course, nearly all of these errors are trivial and non-functional.
But, the trival errors show up on the Checkstyle reports too, so that
it is difficult or impossible to see the forrest for the trees. With
twenty thousand errors blocking our view,  the Checkstyle reports have
little or no practical value.

If we are going to use Checkstyle, then we should use it, and do
whatever we can to bring the project back in line with the Checkstyle
standards. This will cause some pain in the short run, but it is pain
that we brought upon ourselves through inattention.

If we are going to let all twenty thousand errors stand, then perhaps
we should supress the Checkstyle report from the Classic subprojects,
and endeavor to do better on the newer subprojects.

-Ted.

>
> Niall
>
> ----- Original Message -----
> From: <[EMAIL PROTECTED]>
> To: <[EMAIL PROTECTED]>
> Sent: Saturday, January 21, 2006 12:21 AM
> Subject: svn commit: r370938 [1/50] - in /struts: action/trunk/
> action/trunk/conf/java/ action/trunk/src/java/org/apache/struts/
> action/trunk/src/java/org/apache/struts/action/
> action/trunk/src/java/org/apache/struts/chain/
> action/trunk/src/java/org/apache/stru...
>
>
> > Author: husted
> > Date: Fri Jan 20 16:19:02 2006
> > New Revision: 370938
> >
> > URL: http://svn.apache.org/viewcvs?rev=370938&view=rev
> > Log:
> > CheckStyle pass
> > * Reformat with Jalopy and IDEA
> > * No code or content changes
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>


--
HTH, Ted.
http://www.husted.com/poe/

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to