ginp-developers

[Ginp-developers] Project Quality

[Doug C. <do...@cu...>]

Dear Brian,

After your points about code style I thought I would check in a bit of
Maven QA reporting.  This produces reports that are not good for my ego
but they are a great way to measure code quality and the quality
improvements if any...

Update with
> svn up

Generate site locally using:
> mvn site

Check out reports at: ginp/target/site/project-reports.html

Let me know if we really want this in the live site....?  I am happy to
publish and I think it will motivate me to improve quality.

Also let me know if you want the code formated automatically with Jalopy?

All the best,

Doug

Re: [Ginp-developers] Project Quality

[Brian B. <br...@Pi...>]

Doug Culnane wrote:
> Dear Brian,
> 
> After your points about code style I thought I would check in a bit of
> Maven QA reporting.  This produces reports that are not good for my ego
> but they are a great way to measure code quality and the quality
> improvements if any...
> 
> Update with
>> svn up
> 
> Generate site locally using:
>> mvn site
> 
> Check out reports at: ginp/target/site/project-reports.html

It worked very well, although I had to run it from a command line.

This was because my Netbeans maven2 had problems finding some 
dependencies when running the custom goal called "site". I'll worry 
about it another time...

> Let me know if we really want this in the live site....?  I am happy to
> publish and I think it will motivate me to improve quality.

No, I don't think this needs to be on the main site until you feel we 
are ready to publish an 0.30 war for download. At that time, I don't see 
a problem confessing to bad style. As you say, with the reports as a 
reminder, it encourages us to do a bit extra whenever working on a 
particular module. No need for explicit objectives, just good intentions.

Of course, now that I can run the reports locally, I'll be able to do 
something myself.

> Also let me know if you want the code formated automatically with Jalopy?

I didn't know about Jalopy, but have just looked at the site at SourceForge.

Q: would you add it to the maven pom.xml so it is run at every build, or 
just a one-time process?

Q: what style would you impose on the project? (I tend to use the Rogue 
Wave Java Style book as my base reference, then modify my code style 
where necessary to fit that of the project I'm working on.)

Brian

Re: [Ginp-developers] Project Quality

[Doug C. <do...@cu...>]

Brian Burch wrote:
> Q: would you add it to the maven pom.xml so it is run at every build, or 
> just a one-time process?
>   
Yes it goes in the "<build>" section of the POM.  I would bind this in
so it runs on every install phase.
> Q: what style would you impose on the project? (I tend to use the Rogue 
> Wave Java Style book as my base reference, then modify my code style 
> where necessary to fit that of the project I'm working on.)
>   
Not sure.  Jalopy is very configurable but it "ships with sensible
default settings (mimicking the Sun Java coding convention)."  I would
use these. 

I could set up Jalopy and you can try it with the command. 
> mvn install

This will format all code.  I will check in Jalopy in the POM and all
the formatted code.  This will be a big change in the subversion
repository, so please tell me when you have everything checked in and
then I will do this.  After that if "mvn install" is run regularly then
the formatting will be stable and only new stuff will get altered. 
However you need to be aware that you and your formatter are changing
code and this can get confusing and create SVN conflicts.

The problem with setting up an IDE is that a project uses multiple
IDEs.  JUnit, Eclipse and Netbeans have now all be used for the GINP. 
Getting these set up the same is not easy.

An alternative is to run Jalopy once from the command line, but this
need to be done regularly.

Another alternative is not to do any formatting but this also has problems.

Let me know what you want?

All the best,

Doug

Re: [Ginp-developers] Project Quality

[Brian B. <br...@Pi...>]

Doug Culnane wrote:
> Brian Burch wrote:
>> Q: would you add it to the maven pom.xml so it is run at every build, or 
>> just a one-time process?
> Yes it goes in the "<build>" section of the POM.  I would bind this in
> so it runs on every install phase.
>> Q: what style would you impose on the project? (I tend to use the Rogue 
>> Wave Java Style book as my base reference, then modify my code style 
>> where necessary to fit that of the project I'm working on.)
>>   
> Not sure.  Jalopy is very configurable but it "ships with sensible
> default settings (mimicking the Sun Java coding convention)."  I would
> use these. 
> 
> I could set up Jalopy and you can try it with the command. 
> mvn install
> This will format all code.  I will check in Jalopy in the POM and all
> the formatted code.  This will be a big change in the subversion
> repository, so please tell me when you have everything checked in and
> then I will do this.  After that if "mvn install" is run regularly then
> the formatting will be stable and only new stuff will get altered. 
> However you need to be aware that you and your formatter are changing
> code and this can get confusing and create SVN conflicts.
> 
> The problem with setting up an IDE is that a project uses multiple
> IDEs.  JUnit, Eclipse and Netbeans have now all be used for the GINP. 
> Getting these set up the same is not easy.
> 
> An alternative is to run Jalopy once from the command line, but this
> need to be done regularly.

Thanks for explaining. I think it is not worth the extra complexity to 
setup and run Jalopy on every build. I can't imagine us suddenly 
attracting a large number of active and anarchic developers...

I think the best approach is for you to run Jalopy against your own 
sandbox with the Sun style (if that suits you best). You can then take a 
look at the reports and source code it produces - and whether the build 
still works successfully. Assuming you don't have any big problems and 
you like the results, then just commit the cleaned-up modules. (It might 
be worth tagging before and after - or is that just "cvs thinking"?)

Let me have a reference to whatever style you adopt. I'll run an update 
against my sandbox to collect your changes and simply(?) follow the new 
coding style for any future changes I make.

> Another alternative is not to do any formatting but this also has problems.

No, I think it is probably worth the one-time effort to tidy the code. 
If you do the Jalopy stuff, I'll reciprocate (when you have finished!) 
by fixing some of the more embarrassing problems from the code quality 
reports.

I didn't plan to make any more changes for a week or so. If you want to 
do the Jalopy run further away than that, just let me know so I can 
commit any pending changes before you start. Once you start running 
Jalopy, I'll consider the repository frozen until you tell me you have 
finished with it.

Regards,

Brian

Re: [Ginp-developers] Project Quality

[Doug C. <do...@cu...>]

Done a one time format of all code with Japloy default settings, and
checked it in.

All the best,

Doug

Re: [Ginp-developers] Project Quality

[Brian B. <br...@Pi...>]

Doug Culnane wrote:
> Done a one time format of all code with Japloy default settings, and
> checked it in.

Fast work! My svn update collected a lot of changed source.

I looked at a random source and thought the default jalopy settings were 
a bit strange...

- two blanks for any empty argument list.

- some very long lines that will get warnings in the style checks.

- argument lists split, but aligned too far over on the right.


In general, I find it very readable.. and the tabs have gone so there 
aren't any statement continuations hidden off the right of my page.

However, the Checkstyle report claims there are 788 errors. Although 
most are to do with javadoc, there are quite a lot that are not. Should 
we change the rules for Jalopy or Checkstyle?

Do you want to fine-tune, or just leave things alone?

Either is OK with me, but I won't start the code quality work until you 
say you have finished formatting.

Thanks,

Brian

Re: [Ginp-developers] Project Quality

[Doug C. <do...@cu...>]

Dear Brian,

I have a look at Jalopy and the maven plugin and it has got a bit more
complicated than it used to be.  Basically I have lost patience with
it.  I can not find a SUN standard configuration so the best match is
the Maven coding conventions and the Jalopy standard.  It used to be a
bit easier to use but now it has gone all wizard, gui, product,
commercial etc... and I therefore it is beyond me to be able to
integrate it into the project as i used to do.

I can switch to the SUN standard for checkstyle but this complains a lot
about the use of {()} etc....

Therefore I would suggest that we leave it as it is and new code is
formated manually.  Thank Jalopy for doing 95% of the formating for us
and the rest we do by hand as and when we can be bothered.  The
Checkstyle Maven standard is the authority on what is and is not well
formatted.  Maven issue a formatter config for different IDEs
(http://maven.apache.org/guides/development/guide-m2-development.html#Maven%20Code%20Style). 


Is this acceptable to you, or do you have a better idea.?

note
{
    JavaDocs have to be done by had but maybe your IDE can help.
}


All the best,

Doug

Brian Burch wrote:
> Doug Culnane wrote:
>   
>> Done a one time format of all code with Japloy default settings, and
>> checked it in.
>>     
>
> Fast work! My svn update collected a lot of changed source.
>
> I looked at a random source and thought the default jalopy settings were 
> a bit strange...
>
> - two blanks for any empty argument list.
>
> - some very long lines that will get warnings in the style checks.
>
> - argument lists split, but aligned too far over on the right.
>
>
> In general, I find it very readable.. and the tabs have gone so there 
> aren't any statement continuations hidden off the right of my page.
>
> However, the Checkstyle report claims there are 788 errors. Although 
> most are to do with javadoc, there are quite a lot that are not. Should 
> we change the rules for Jalopy or Checkstyle?
>
> Do you want to fine-tune, or just leave things alone?
>
> Either is OK with me, but I won't start the code quality work until you 
> say you have finished formatting.
>
> Thanks,
>
> Brian
>
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share your
> opinions on IT & business topics through brief surveys - and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> _______________________________________________
> Ginp-developers mailing list
> Gin...@li...
> https://lists.sourceforge.net/lists/listinfo/ginp-developers
>   

Re: [Ginp-developers] Project Quality

[Brian B. <br...@Pi...>]

Doug Culnane wrote:
> I have a look at Jalopy and the maven plugin and it has got a bit more
> complicated than it used to be.  Basically I have lost patience with
> it.  I can not find a SUN standard configuration so the best match is
> the Maven coding conventions and the Jalopy standard.  It used to be a
> bit easier to use but now it has gone all wizard, gui, product,
> commercial etc... and I therefore it is beyond me to be able to
> integrate it into the project as i used to do.
> 
> I can switch to the SUN standard for checkstyle but this complains a lot
> about the use of {()} etc....
> 
> Therefore I would suggest that we leave it as it is and new code is
> formated manually.  Thank Jalopy for doing 95% of the formating for us
> and the rest we do by hand as and when we can be bothered.  The
> Checkstyle Maven standard is the authority on what is and is not well
> formatted.  Maven issue a formatter config for different IDEs
> (http://maven.apache.org/guides/development/guide-m2-development.html#Maven%20Code%20Style). 
> 
> 
> Is this acceptable to you, or do you have a better idea.?

Yes, that's fine.

We'll take the Jalopy formatting as our "new" starting point. Any work I 
do on the code cleanup will be acceptable as long as it reduces the 
number of maven complaints.

Thanks for taking a sensible decision. We don't want to make work for 
ourselves!

Brian

Re: [Ginp-developers] Project Quality

[Brian B. <br...@Pi...>]

Well, things didn't work out quite so well in practice. I did a "mvn 
site" against your latest source and CheckStyle threw out more than 7800 
errors!

I took ~/commands/Logon.java as a typical example and started to fix the 
errors manually. Fixing one would usually cause another to pop out and 
replace it. By the time I'd spent an hour, I hit a major snag because we 
did not have the Apache Copyright statement in our source!!! I realised 
there was something fundamental going wrong. How could we be using and 
enforcing Sun coding conventions if we needed an Apache copyright?

Well, several days later I've worked a lot of this out. It is a pity 
there isn't better documentation on the web - or if there is, I couldn't 
find it with google.

I followed your tracks to start with. I found out how to run Jalopy and 
decided the only sensible option was a single-shot from the 
command-line. Eventually, I found some instructions on how to create my 
own Jalopy configuration file by running the preferences.sh gui; 
incorporate our own copyright statement; then export the xml file. I 
then ran Jalopy against the source - I couldn't get recursion to work so 
I ran it directory-by-directory.

In the middle of my Jalopy experiments, I kept running "mvn 
checkstyle:checkstyle" to monitor my progress. CheckStyle did not appear 
to be applying the Sun coding standards - at least not the same ones as 
Jalopy!

Eventually I took a CVS snapshot of sun-checks.xml from the CheckStyle 
source and modified pom.xml to use it. That wasn't any good because it 
needed a valid DTD. I found the correct doctype statement, pasted it in 
and checkstyle ran OK with a BIG reduction in the error count.


So, here is the situation. I have used Jalopy to reformat all the source 
according to explicit Sun standards. It looks MUCH better to my eye, 
e.g. opening curlies on the same line, fewer spaces in argument lists, 
etc. It might not be perfect, but it is good enough to live with. Sorry 
to say that every source module now looks different to the versions that 
you checked-in last week...

I've also put my jalopy-ginp.xml file into the source tree, even though 
it isn't (and shouldn't) be used by maven, it is worth keeping for 
documentation purposes. Besides, we might want to tinker with it in 
future (or rip it off for other projects).

I've put my working checkstyle-ginp.xml file in the source tree too, and 
now pom.xml uses it when running CheckStyle.

Currently, we still have 1404 style errors after a clean/test/site 
sequence. I've reviewed quite a few and they appear to be reasonable and 
worth fixing. I won't go into detail, but some are of more concern than 
others.

I'm ready to commit all these changes. I intend to do it in two chunks - 
the xml's first, then all the updated java source files. I don't think 
it is worth the effort of regressing your changes, so I would commit 
against the latest version. Are you happy for me to go ahead?

Regards,

Brian

p.s. I haven't yet run Jalopy against the test classes, but would do 
once the main classes were mostly cleaned up.

Re: [Ginp-developers] Project Quality

[Brian B. <br...@Pi...>]

Doug Culnane wrote:
> You have more patience than I do.  Jalopy and checkstyle are very cool
> but there is a lot of hassle to get the most out of them.  I have done
> this before but Jalopy has changed since then and the Maven plug-in is
> no longer available except in source format.

I wouldn't call it patience - more like "too stubborn to give up".

> Excellent work Brian.  Please do check it in.

As you will see, I checked in all the source and xml files after the 
jalopy run.

Next, I went through and fixed every CheckStyle line length error. The 
system is now down to "only" 1290 style errors. I've committed all these 
changes too.

I hope to have a couple of spare hours over the weekend. I think I will 
stop working "vertically" to resolve style errors - in future I'll 
select a single source file and clean it up as much as possible in a 
single hit. I'm not going to be obsessive and fix everything - if it 
isn't quick or serious, I'll leave it until another time. I want to get 
the error count down to a low number (whatever that might be!)

Regards,

Brian