You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by Paul Sundling <to...@tkz.net> on 2004/01/10 23:23:10 UTC
maven checkstyle report
I was thinking that clearing up some of the most common checkstyle
errors might make those reports more useful. There are two errors which
would be broadly fixed with little problem. This would vastly improve
the signal to noise ratio on the report. :)
1. License file check errors: There is a check to make sure all the
source code begins with the license. This is generating errors because
the copyrights don't all follow the same years as what it expects to
find. It does lead me to question how the years in the copyrights are
updated. This is now 2004 and I wonder if any files have 2004 in them
for instance. There are at least two approachs to fix the checkstyle issue:
Option 1 (easy way) we modify conf/qa/CheckStyle.xml and add 10 to
ignoreLines (already has 2,3,4,5). I can submit a patch for that today
if there is a go ahead.
Option 2 we could make another copy of the license file with a
regular expression for the years in the file.
I'm leaning towards option 1 on this myself and it seems consistent with
solutions that have been applied in the past.
2. Tab errors: It has an error for tabs in the files.
Option 1 we remove the tabs check from checkStyle.
Option 2 we replace tabs with spaces and do a quick visual check to
see how everything lines up.
Which approach to take here isn't as obvious. How important is it
wether there are tabs or spaces? That's not really my call, but I'm
willing to take care of it either way.
Paul Sundling
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org
RE: maven checkstyle report
Posted by Tim Chen <tc...@tampabay.rr.com>.
You can use ant pretty. Let me know if you need an example.
Very easy to use and will enforce a standard across everything not just
tabs.
Tim Chen
tchen@tampabay.rr.com
-----Original Message-----
From: Indrajit Raychaudhuri [mailto:irc@indrajit.com]
Sent: Sunday, January 11, 2004 12:46 AM
To: Struts Developers List
Subject: Re: maven checkstyle report
----- Original Message -----
From: "Paul Sundling" <to...@tkz.net>
To: "Struts Developers List" <st...@jakarta.apache.org>
Sent: Sunday, 11 January, 2004 04:27
Subject: Re: maven checkstyle report
> Actually, I was realizing that Option 2 is only a quick fix, not a
> solution and that eventually there will still be a bunch of tab
> errors. Realizing that we'll be accepting code from people who may or
> may not be using tabs, it's likely to be a persistant error... Some
> assistance could be provided by an ant target like this that I used to
> package my own code for a client who hated tabs:
>
> <target name="notabs" description="Replace tab with 4 spaces" >
> <replaceregexp match="\t"
> replace=" "
> flags="g" >
> <fileset dir="web" excludes="**/images/*" />
> </replaceregexp>
> </target>
>
> Even that assumes a tab is always the same number of spaces, which is
> the whole issue with tabs in the first place.
You could also use the Ant task <fixcrlf/>
For example (assuming Struts convention of '4 whitespaces per tab'), we
could have
<fixcrlf
srcdir="${src.java.dir}"
tab="remove"
tablength="4"/>
- Indrajit
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org
Re: maven checkstyle report
Posted by Indrajit Raychaudhuri <ir...@indrajit.com>.
----- Original Message -----
From: "Paul Sundling" <to...@tkz.net>
To: "Struts Developers List" <st...@jakarta.apache.org>
Sent: Sunday, 11 January, 2004 04:27
Subject: Re: maven checkstyle report
> Actually, I was realizing that Option 2 is only a quick fix, not a
> solution and that eventually there will still be a bunch of tab errors.
> Realizing that we'll be accepting code from people who may or may not be
> using tabs, it's likely to be a persistant error... Some assistance
> could be provided by an ant target like this that I used to package my
> own code for a client who hated tabs:
>
> <target name="notabs" description="Replace tab with 4 spaces" >
> <replaceregexp match="\t"
> replace=" "
> flags="g" >
> <fileset dir="web" excludes="**/images/*" />
> </replaceregexp>
> </target>
>
> Even that assumes a tab is always the same number of spaces, which is
> the whole issue with tabs in the first place.
You could also use the Ant task <fixcrlf/>
For example (assuming Struts convention of '4 whitespaces per tab'), we
could have
<fixcrlf
srcdir="${src.java.dir}"
tab="remove"
tablength="4"/>
- Indrajit
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org
Re: maven checkstyle report
Posted by Paul Sundling <to...@tkz.net>.
Actually, I was realizing that Option 2 is only a quick fix, not a
solution and that eventually there will still be a bunch of tab errors.
Realizing that we'll be accepting code from people who may or may not be
using tabs, it's likely to be a persistant error... Some assistance
could be provided by an ant target like this that I used to package my
own code for a client who hated tabs:
<target name="notabs" description="Replace tab with 4 spaces" >
<replaceregexp match="\t"
replace=" "
flags="g" >
<fileset dir="web" excludes="**/images/*" />
</replaceregexp>
</target>
Even that assumes a tab is always the same number of spaces, which is
the whole issue with tabs in the first place. I personally like tabs,
but not as much as I like consistency. :)
Paul
Joe Germuska wrote:
>> 2. Tab errors: It has an error for tabs in the files. Option 1 we
>> remove the tabs check from checkStyle.
>> Option 2 we replace tabs with spaces and do a quick visual check to
>> see how everything lines up.
>>
>> Which approach to take here isn't as obvious. How important is it
>> wether there are tabs or spaces? That's not really my call, but I'm
>> willing to take care of it either way.
>
>
> This is the stuff religious wars are made of; it is true, though,
> things like diff work much better if indentation is done consistently,
> whichever character is used. I don't recall if there's an official
> preference in Struts, but I imagine someone will pipe in if it's
> important to them.
>
> If it turns into a big debate, better to disable the tabs check,
> because as you say, it'll improve the signal-to-noise ratio in the
> checkstyle reports.
>
> Joe
>
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org
Re: maven checkstyle report
Posted by David Graham <gr...@yahoo.com>.
--- Joe Germuska <Jo...@Germuska.com> wrote:
> >1. License file check errors: There is a check to make sure all the
> >source code begins with the license. This is generating errors
> >because the copyrights don't all follow the same years as what it
> >expects to find. It does lead me to question how the years in the
> >copyrights are updated. This is now 2004 and I wonder if any files
> >have 2004 in them for instance. There are at least two approachs to
> >fix the checkstyle issue:
> > Option 1 (easy way) we modify conf/qa/CheckStyle.xml and add 10 to
> >ignoreLines (already has 2,3,4,5). I can submit a patch for that
> >today if there is a go ahead.
> > Option 2 we could make another copy of the license file with a
> >regular expression for the years in the file.
> >
> >I'm leaning towards option 1 on this myself and it seems consistent
> >with solutions that have been applied in the past.
>
> I agree, option 1 is much simpler.
>
> >2. Tab errors: It has an error for tabs in the files. Option 1 we
> >remove the tabs check from checkStyle.
> > Option 2 we replace tabs with spaces and do a quick visual check
> >to see how everything lines up.
> >
> >Which approach to take here isn't as obvious. How important is it
> >wether there are tabs or spaces? That's not really my call, but I'm
> >willing to take care of it either way.
>
> This is the stuff religious wars are made of; it is true, though,
> things like diff work much better if indentation is done
> consistently, whichever character is used. I don't recall if there's
> an official preference in Struts, but I imagine someone will pipe in
> if it's important to them.
>
> If it turns into a big debate, better to disable the tabs check,
> because as you say, it'll improve the signal-to-noise ratio in the
> checkstyle reports.
We need to keep the tab check because we only use spaces in Struts code.
David
>
> Joe
>
> --
> Joe Germuska
> Joe@Germuska.com
> http://blog.germuska.com
> "We want beef in dessert if we can get it there."
> -- Betty Hogan, Director of New Product Development, National
> Cattlemen's Beef Association
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: struts-dev-help@jakarta.apache.org
>
__________________________________
Do you Yahoo!?
Yahoo! Hotjobs: Enter the "Signing Bonus" Sweepstakes
http://hotjobs.sweepstakes.yahoo.com/signingbonus
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org
Re: maven checkstyle report
Posted by Ted Husted <hu...@apache.org>.
On Sat, 10 Jan 2004 16:24:49 -0600, Joe Germuska wrote:
> This is the stuff religious wars are made of; it is true, though,
> things like diff work much better if indentation is done
> consistently, whichever character is used. I don't recall if
> there's an official preference in Struts, but I imagine someone
> will pipe in if it's important to them.
So far, we've started with the Jakarta Guidelines and then add our own references and clarifications.
http://jakarta.apache.org/struts/releases.html#Coding
As mentioned, the tab issue is related to sending automated update alerts by email.
-Ted.
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org
Re: maven checkstyle report
Posted by Joe Germuska <Jo...@Germuska.com>.
>1. License file check errors: There is a check to make sure all the
>source code begins with the license. This is generating errors
>because the copyrights don't all follow the same years as what it
>expects to find. It does lead me to question how the years in the
>copyrights are updated. This is now 2004 and I wonder if any files
>have 2004 in them for instance. There are at least two approachs to
>fix the checkstyle issue:
> Option 1 (easy way) we modify conf/qa/CheckStyle.xml and add 10 to
>ignoreLines (already has 2,3,4,5). I can submit a patch for that
>today if there is a go ahead.
> Option 2 we could make another copy of the license file with a
>regular expression for the years in the file.
>
>I'm leaning towards option 1 on this myself and it seems consistent
>with solutions that have been applied in the past.
I agree, option 1 is much simpler.
>2. Tab errors: It has an error for tabs in the files. Option 1 we
>remove the tabs check from checkStyle.
> Option 2 we replace tabs with spaces and do a quick visual check
>to see how everything lines up.
>
>Which approach to take here isn't as obvious. How important is it
>wether there are tabs or spaces? That's not really my call, but I'm
>willing to take care of it either way.
This is the stuff religious wars are made of; it is true, though,
things like diff work much better if indentation is done
consistently, whichever character is used. I don't recall if there's
an official preference in Struts, but I imagine someone will pipe in
if it's important to them.
If it turns into a big debate, better to disable the tabs check,
because as you say, it'll improve the signal-to-noise ratio in the
checkstyle reports.
Joe
--
Joe Germuska
Joe@Germuska.com
http://blog.germuska.com
"We want beef in dessert if we can get it there."
-- Betty Hogan, Director of New Product Development, National
Cattlemen's Beef Association
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org
Re: maven checkstyle report
Posted by Ted Husted <hu...@apache.org>.
On Sat, 10 Jan 2004 14:59:22 -0800 (PST), Martin Cooper wrote:
>
> The Struts rule is, and has always been, spaces only. No tabs.
The rule stems from echoing the CVS diffs by email to the DEV list. Tabs confound the emails.
-Ted.
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org
Re: maven checkstyle report
Posted by Martin Cooper <ma...@apache.org>.
On Sat, 10 Jan 2004, Paul Sundling wrote:
> I was thinking that clearing up some of the most common checkstyle
> errors might make those reports more useful. There are two errors which
> would be broadly fixed with little problem. This would vastly improve
> the signal to noise ratio on the report. :)
>
> 1. License file check errors: There is a check to make sure all the
> source code begins with the license. This is generating errors because
> the copyrights don't all follow the same years as what it expects to
> find. It does lead me to question how the years in the copyrights are
> updated. This is now 2004 and I wonder if any files have 2004 in them
> for instance. There are at least two approachs to fix the checkstyle issue:
> Option 1 (easy way) we modify conf/qa/CheckStyle.xml and add 10 to
> ignoreLines (already has 2,3,4,5). I can submit a patch for that today
> if there is a go ahead.
> Option 2 we could make another copy of the license file with a
> regular expression for the years in the file.
>
> I'm leaning towards option 1 on this myself and it seems consistent with
> solutions that have been applied in the past.
I'd prefer that we switch the license (header) check to use a regexp
header rather than a static one. That's what I've done in my "day" jobs,
and I think it's appropriate here too. I know it's a bit more of a pain to
set up initially, but it is a one-off deal.
>
> 2. Tab errors: It has an error for tabs in the files.
> Option 1 we remove the tabs check from checkStyle.
> Option 2 we replace tabs with spaces and do a quick visual check to
> see how everything lines up.
>
> Which approach to take here isn't as obvious. How important is it
> wether there are tabs or spaces? That's not really my call, but I'm
> willing to take care of it either way.
The Struts rule is, and has always been, spaces only. No tabs.
--
Martin Cooper
>
> Paul Sundling
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: struts-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org