You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adam Heath <do...@brainfood.com> on 2010/02/05 07:41:47 UTC
style issues(show me the code)
I'm working on some stuff that will encapsulate the style issues I
have been commenting on. Then, if the styles are not actually what
others want, we can change the code. Then, developers can run the
tools against their changes, to see if they pass muster. This is a
way to automate it; let the computer do what it does best.
Currently, I'm implementing a java diff algorithm; I tried to find one
that is compatible with apache, but everything seems to just want to
reimplement GNU diff; I'm reading *just*
http://c2.com/cgi/wiki?DiffAlgorithm as a base.
Once I have that, I'll then start working on an xslt, one for each xml
schema(possible shared), which will then read the source xml, output
new xml formatted appropriately, then diff to the original, and show
the output. This will require developers to be able to read diff
output, but that isn't hard.
I'll also be looking for java code formatting tools; anyone have any
suggestions that are apache compatible(has to be free, won't use
commercial things at all(don't get me started on confluence)).
Re: style issues(show me the code)
Posted by Jacques Le Roux <ja...@free.fr>.
This sounds like the best proposition on this subject so far
Jacques
From: "Jacopo Cappellato" <ja...@hotwaxmedia.com>
> On Feb 5, 2010, at 10:47 PM, Adam Heath wrote:
>
>>
>> checkstyle(which is the LGPL thing being discussed) needs to be run on
>> each developers workstation; that's the best place to have it
>> effective. If each developer were required to download the library,
>> then that would be a pain in the ass.
>
> Could we add a pre commit hook to check the style of commits?
>
> Jacopo
>
Re: style issues(show me the code)
Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On Feb 5, 2010, at 10:47 PM, Adam Heath wrote:
>
> checkstyle(which is the LGPL thing being discussed) needs to be run on
> each developers workstation; that's the best place to have it
> effective. If each developer were required to download the library,
> then that would be a pain in the ass.
Could we add a pre commit hook to check the style of commits?
Jacopo
Re: style issues(show me the code)
Posted by Ean Schuessler <ea...@brainfood.com>.
Raj Saini wrote:
> For code coverage you can use Clover which is commercial but they give
> the free developer license to open source project similar to the
> IntelliJ IDEA and it is also used in other Apache projects.
>
> http://tapestry.apache.org/tapestry4/tapestry-contrib/clover/index.html
We've been using Cobertura internally which is Free Software (though
GPL). The main problem for OFBiz with regard to code coverage is that
none of the coverage tools will show you the utilization of simple
methods, which constitute a big chunk of OFBiz' behavior.
--
Ean Schuessler, CTO
ean@brainfood.com
214-720-0700 x 315
Brainfood, Inc.
http://www.brainfood.com
Re: style issues(show me the code)
Posted by Raj Saini <ra...@gmail.com>.
There are other Apache projects using checkstyle but not sure how. See
the tiles link below.
http://tiles.apache.org/2.0/framework/tiles-jsp/checkstyle.html
For code coverage you can use Clover which is commercial but they give
the free developer license to open source project similar to the
IntelliJ IDEA and it is also used in other Apache projects.
http://tapestry.apache.org/tapestry4/tapestry-contrib/clover/index.html
Thanks,
Raj
Adam Heath wrote:
> Ean Schuessler wrote:
>
>> Adam Heath wrote:
>>
>>> Sorry, no, it's LGPL.
>>>
>>>
>> Do the test suites have to be checked in to the repository? We do use
>> JIRA for bugtracking.
>>
>
> checkstyle(which is the LGPL thing being discussed) needs to be run on
> each developers workstation; that's the best place to have it
> effective. If each developer were required to download the library,
> then that would be a pain in the ass.
>
> This same reason is why I haven't pushed coverage reports(using
> cobertura, which is GPL) more. I am open to other tools, that have a
> java-based api I can hook into, if others know of any. But, before
> anything gets added to the ofbiz repository, it must be an apache
> compatible license. Just saying that it's installed on some other
> apache-infra type machine is not good enough, imho.
>
>
>
>
>
Re: style issues(show me the code)
Posted by Adam Heath <do...@brainfood.com>.
Ean Schuessler wrote:
> Adam Heath wrote:
>> Sorry, no, it's LGPL.
>>
> Do the test suites have to be checked in to the repository? We do use
> JIRA for bugtracking.
checkstyle(which is the LGPL thing being discussed) needs to be run on
each developers workstation; that's the best place to have it
effective. If each developer were required to download the library,
then that would be a pain in the ass.
This same reason is why I haven't pushed coverage reports(using
cobertura, which is GPL) more. I am open to other tools, that have a
java-based api I can hook into, if others know of any. But, before
anything gets added to the ofbiz repository, it must be an apache
compatible license. Just saying that it's installed on some other
apache-infra type machine is not good enough, imho.
>
Re: style issues(show me the code)
Posted by Ean Schuessler <ea...@brainfood.com>.
Adam Heath wrote:
> Sorry, no, it's LGPL.
>
Do the test suites have to be checked in to the repository? We do use
JIRA for bugtracking.
--
Ean Schuessler, CTO
ean@brainfood.com
214-720-0700 x 315
Brainfood, Inc.
http://www.brainfood.com
Re: style issues(show me the code)
Posted by Adam Heath <do...@brainfood.com>.
Erwan de FERRIERES wrote:
> Le 05/02/2010 17:20, Adam Heath a écrit :
>
>> pps: Upon further scanning, PMD doesn't appear to do space checks.
>> Bother. So I am still looking for something to do that.
>>
> space checks can be made by checkstyle :
> http://checkstyle.sourceforge.net/config_whitespace.html
Sorry, no, it's LGPL.
Re: style issues(show me the code)
Posted by Erwan de FERRIERES <er...@nereide.fr>.
Le 05/02/2010 17:20, Adam Heath a écrit :
> pps: Upon further scanning, PMD doesn't appear to do space checks.
> Bother. So I am still looking for something to do that.
>
space checks can be made by checkstyle :
http://checkstyle.sourceforge.net/config_whitespace.html
--
Erwan de FERRIERES
www.nereide.biz
Re: style issues(show me the code)
Posted by Adam Heath <do...@brainfood.com>.
Adam Heath wrote:
> Scott Gray wrote:
>> On 4/02/2010, at 11:41 PM, Adam Heath wrote:
>>
>>> I'll also be looking for java code formatting tools; anyone have any
>>> suggestions that are apache compatible(has to be free, won't use
>>> commercial things at all(don't get me started on confluence)).
>> PMD (http://pmd.sourceforge.net/) uses a BSD style license and may be worth a look.
>
> Hmm. Looking at controversial rulesets makes me interested; I bet the
> other rulesets are equally enjoyable. Thanks, will try it out.
Oh, PMD, how I love thee, let me count the ways. Oh, whoops, I just
had a StackOverFlow. I guess that means I've finally found my true
love. Let's get married and make beautiful babies together. Then,
they can all become leaders of all the countries of the world, and we
will achieve world dominiation.
In a word, I <blink>LOVE</blink> PMD.
Based on the example rules I read about on their site, here are some
things that we could scan for(code issues):
==
String var;
// this should use UtilValidate
if (var != null && var.length > 0);
==
Collection var;
// this should use UtilValidate
if (var != null && var.isEmpty());
==
public boolean equals(Object o) {
// This throws a ClassCastException, which is not
// what the contract for this method allows.
MyClass other = (MyClass) o;
==
// This loop should stay an iterator/while combo. PMD
// can detect the remove call. Any other loops can be
// recommended for using enhanced-for
Iterator it = collection.iterator();
while (it.hasNext()) {
// code block
it.remove();
// code block
}
==
// should use valueOf
new Integer(5);
==
// should be new BigDecimal(".1")
new BigDecimal(.1);
==
// when pulling from the database, the string variant
// should be used, not the double variant. double
// values are not exact.
double d = .1;
new BigDecimal(d);
==
// don't concat vars in append() calls
new StringBuilder().append("foo" + 1 + "bar");
==
ps: The BigDecimal checks are not something I knew about. Those
actually already exist for PMD, and I was surprised to read about
them.
http://pmd.sourceforge.net/rules/basic.html#AvoidDecimalLiteralsInBigDecimalConstructor
pps: Upon further scanning, PMD doesn't appear to do space checks.
Bother. So I am still looking for something to do that.
Re: style issues(show me the code)
Posted by Adam Heath <do...@brainfood.com>.
Scott Gray wrote:
> On 4/02/2010, at 11:41 PM, Adam Heath wrote:
>
>> I'll also be looking for java code formatting tools; anyone have any
>> suggestions that are apache compatible(has to be free, won't use
>> commercial things at all(don't get me started on confluence)).
>
> PMD (http://pmd.sourceforge.net/) uses a BSD style license and may be worth a look.
Hmm. Looking at controversial rulesets makes me interested; I bet the
other rulesets are equally enjoyable. Thanks, will try it out.
Re: style issues(show me the code)
Posted by Erwan de FERRIERES <er...@nereide.fr>.
Le 05/02/2010 08:33, Scott Gray a écrit :
> On 4/02/2010, at 11:41 PM, Adam Heath wrote:
>
>> I'll also be looking for java code formatting tools; anyone have any
>> suggestions that are apache compatible(has to be free, won't use
>> commercial things at all(don't get me started on confluence)).
>
> PMD (http://pmd.sourceforge.net/) uses a BSD style license and may be worth a look.
>
> Regards
> Scott
I have already the PMD task done, just add those lines in the build.xml
file, and add the relevant libraries in framework/base/lib
<path id="cq.classpath">
<pathelement location="."/>
<fileset dir="framework/base/lib/">
<include name="*.jar"/>
</fileset>
</path>
<target name="pmd">
<taskdef name="pmd"
classname="net.sourceforge.pmd.ant.PMDTask" classpathref="cq.classpath"/>
<pmd shortFilenames="true" rulesetfiles="basic">
<formatter type="xml" toFile="runtime/pmd.xml" />
<fileset dir="." includes="**/*.java" />
</pmd>
</target>
You can also use checkstyle, which will compare code to rules
(http://checkstyle.sourceforge.net)
--
Erwan de FERRIERES
www.nereide.biz
Re: style issues(show me the code)
Posted by Scott Gray <sc...@hotwaxmedia.com>.
On 4/02/2010, at 11:41 PM, Adam Heath wrote:
> I'll also be looking for java code formatting tools; anyone have any
> suggestions that are apache compatible(has to be free, won't use
> commercial things at all(don't get me started on confluence)).
PMD (http://pmd.sourceforge.net/) uses a BSD style license and may be worth a look.
Regards
Scott
Re: style issues(show me the code)
Posted by Adam Heath <do...@brainfood.com>.
Adam Heath wrote:
> Currently, I'm implementing a java diff algorithm; I tried to find one
> that is compatible with apache, but everything seems to just want to
> reimplement GNU diff; I'm reading *just*
> http://c2.com/cgi/wiki?DiffAlgorithm as a base.
Hmm. Ean just called me to talk about work stuff, and he suggested I
look at eclipse. While the license of eclipse may be questionable(can
other comment on that), it does give me some more things to do google
searches on; any java ide that can show diffs. Again, others can
offer suggestions here.