+1 If we are going to have some coding standards then let's enforce
them. That might make us actually figure out how to get Eclipse and
IntelliJ formatting in synch. There are also a number of code quality
rules that can be enabled in these tools (I'm most familiar with those
in Eclipse) and publishing a standard set of these options for each tool
in addition to the formatting conventions would be a big improvement.
Enforcing PMD and checkstyle consistency in maven seems like a useful
bar to set but can this be enabled for new checkins only? I'm afraid we
would never get a clean build if any of the current issues becomes a
show-stopper.
On 6/10/12 10:17 PM, Robin Anil wrote:
Do it!
On Jun 10, 2012 9:09 PM, "Benson Margulies"<[email protected]> wrote:
I hesitate to remind you all that the maven plugins can be wired up
for at least checkstyle and PMD as parts of the build that *fail*, not
just report, and that several other Apache projects live very happily
this way. This makes it pretty nearly impossible to check in code that
doesn't meet whatever standards are configured.
On Sun, Jun 10, 2012 at 5:19 PM, Robin Anil<[email protected]> wrote:
Grant, you mentioned you have some documented steps to hookup Jira patch
submit with jenkins. Can you share those. Findbugs/Checkstyle/Pmd/Clover
is
already integrated in our Jenkins build. I bet we should be able to get
decent stats on each patch. To me that's a more sustainable process after
doing a one time massive fix.
Robin
On Sun, Jun 10, 2012 at 12:03 PM, Grant Ingersoll<[email protected]
wrote:
On Jun 9, 2012, at 7:17 PM, Sean Owen wrote:
Guys, I'm preparing a large new patch that fixes style problems in the
code, for after the code freeze. This is my last pass at this for
Mahout.
Style is not a big deal, though it's probably not good that random
non-standard Java is committed to the project. The only hard 'fix' for
this long-standing phenomenon is requiring a review process, and that
is too much. I don't think this project adheres to standards so much,
and such is life.
Perhaps we should at least clean up style before every release. I've
seen
other projects do this and while it isn't perfect, it does mean that we
start from a clean slate every time.
Naturally, committers can also stylize right before committing, too.
This
usually reduces the burden on the contributor, but keeps the code base
in
good form.
However, simply turning on code inspections in a modern IDE like
IntelilJ is turning up plain bugs in the code. I want to call out a
few, because I want to fix them (after 0.7), but also because I want
to make the point that static analysis can find bugs. Because it can,
it should. I think open source projects can and should be the finest
output of the best and brightest. And at "mere" Google, stuff that
static analysis finds would never have gotten to even code review.
Hence I am somewhat dismayed to see so many problems being committed
without review into the code base.
+1.
-Grant
