Turn on code inspections, please

[Sean Owen <sr...@gmail.com>]

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.

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.

Here's a taste...


TestMeanShift.testCanopyEuclideanMRJob(), line 365

    Vector[] permutedRaw = new Vector[raw.length];
    for (int i = 0; i < raw.length; i++)
      permutedRaw = raw;

Oops. I think this accidentally accomplishes its goal of a copy, but
it intends a deep copy and that's not what it does.



LocalSSVDPCADenseTest.runSSVDSolver(), line 107:

    xi.assign(Functions.mult(1 / m));

m is an integer, so 1/m == 0 for any m > 1. This is not the intent,
but the test is actually crafted to work only when xi is assigned to
0.


Let's raise the bar please.

Re: Turn on code inspections, please

[Hector Yee <he...@gmail.com>]

I agree re: bags of bits of code. e.g there is no single binary that can
try all permutations of various techniques on a single data set and pick
the best model.
instead there seems to be N different binaries for every (classifier)
technique at least circa 0.5

Is there no presubmit checkin hook for svn or git?

On Sun, Jun 10, 2012 at 8:24 AM, Sean Owen <sr...@gmail.com> wrote:

> My point was rather that it shouldn't even reach review, and was more
> suggesting I'm giving up on trying to fix it that continuing.
>
> I do think a review process is not realistic to expect, which is all
> the more reason that if individual committers are going to dump code
> in directly, that it at least pass basic checks available in IDEs.
> This isn't happening here, and despite some discussion, hasn't
> changed.
>
> Ted I think you made an offhand comment that you encountered an
> independent review of the code base and it was viewed as fair-to-poor
> quality. I encounter the same, and I agree. It's a real barrier to the
> project going forward; if you don't 'trust' the code, can't easily
> change or understand the code, it discourages using or building on it.
>
> I am not sure it's truly fixable at this point; such is life. This is
> mostly a bag of bits of people's code. This is why, for example, I'm
> doing my own new development in a separate project. But still worth
> striving for quality, for those that want to continue to work on this
> project.
>
>
> On Sun, Jun 10, 2012 at 12:38 AM, Robin Anil <ro...@gmail.com> wrote:
> > +1
> >
> > Sean you can always keep cleaning this but the root cause is many, and
> > issues like this will slowly creep in over time.
> >
> > One of the root cause is that reviewing is hard in apache at the moment.
> I
> > am really hoping we move to a set of tools that allows any contributor
> just
> > to make changes -> send to review -> (get responses from jenkins) ->
> rework
> > -> resend patch -> get approval from a committer -> submit.
> >
> > If you ask me to inspect a diff submitted on jira with absolutely no time
> > available in day time. I will just look for general correctness. I would
> > like to see static analysis output directly from jenkins, when someone
> > checks in a patch.
> >
> > Robin
> > ------
> > Robin Anil
>



-- 
Yee Yang Li Hector <https://plus.google.com/106746796711269457249>
Professional Profile <http://www.linkedin.com/in/yeehector>
http://hectorgon.blogspot.com/ (tech + travel)
http://hectorgon.com (book reviews)

Re: Turn on code inspections, please

[Sean Owen <sr...@gmail.com>]

My point was rather that it shouldn't even reach review, and was more
suggesting I'm giving up on trying to fix it that continuing.

I do think a review process is not realistic to expect, which is all
the more reason that if individual committers are going to dump code
in directly, that it at least pass basic checks available in IDEs.
This isn't happening here, and despite some discussion, hasn't
changed.

Ted I think you made an offhand comment that you encountered an
independent review of the code base and it was viewed as fair-to-poor
quality. I encounter the same, and I agree. It's a real barrier to the
project going forward; if you don't 'trust' the code, can't easily
change or understand the code, it discourages using or building on it.

I am not sure it's truly fixable at this point; such is life. This is
mostly a bag of bits of people's code. This is why, for example, I'm
doing my own new development in a separate project. But still worth
striving for quality, for those that want to continue to work on this
project.


On Sun, Jun 10, 2012 at 12:38 AM, Robin Anil <ro...@gmail.com> wrote:
> +1
>
> Sean you can always keep cleaning this but the root cause is many, and
> issues like this will slowly creep in over time.
>
> One of the root cause is that reviewing is hard in apache at the moment. I
> am really hoping we move to a set of tools that allows any contributor just
> to make changes -> send to review -> (get responses from jenkins) -> rework
> -> resend patch -> get approval from a committer -> submit.
>
> If you ask me to inspect a diff submitted on jira with absolutely no time
> available in day time. I will just look for general correctness. I would
> like to see static analysis output directly from jenkins, when someone
> checks in a patch.
>
> Robin
> ------
> Robin Anil

Re: Turn on code inspections, please

[Robin Anil <ro...@gmail.com>]

+1

Sean you can always keep cleaning this but the root cause is many, and
issues like this will slowly creep in over time.

One of the root cause is that reviewing is hard in apache at the moment. I
am really hoping we move to a set of tools that allows any contributor just
to make changes -> send to review -> (get responses from jenkins) -> rework
-> resend patch -> get approval from a committer -> submit.

If you ask me to inspect a diff submitted on jira with absolutely no time
available in day time. I will just look for general correctness. I would
like to see static analysis output directly from jenkins, when someone
checks in a patch.

Robin
------
Robin Anil


On Sat, Jun 9, 2012 at 6:17 PM, Sean Owen <sr...@gmail.com> 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.
>
> 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.
>
> Here's a taste...
>
>
> TestMeanShift.testCanopyEuclideanMRJob(), line 365
>
>    Vector[] permutedRaw = new Vector[raw.length];
>    for (int i = 0; i < raw.length; i++)
>      permutedRaw = raw;
>
> Oops. I think this accidentally accomplishes its goal of a copy, but
> it intends a deep copy and that's not what it does.
>
>
>
> LocalSSVDPCADenseTest.runSSVDSolver(), line 107:
>
>    xi.assign(Functions.mult(1 / m));
>
> m is an integer, so 1/m == 0 for any m > 1. This is not the intent,
> but the test is actually crafted to work only when xi is assigned to
> 0.
>
>
> Let's raise the bar please.
>

Re: Turn on code inspections, please

[Jake Mannix <ja...@gmail.com>]

On Mon, Jun 11, 2012 at 7:40 AM, Robin Anil <ro...@gmail.com> wrote:

> ------
> Robin Anil
>
>
> On Mon, Jun 11, 2012 at 9:30 AM, Jeff Eastman <jdog@windwardsolutions.com
> >wrote:
>
> > +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.
> >
> > Does it have a threshold option? Like fail if issue count > 100 (or
> whatever number is there at the moment) and strive to decrease that number
> with each patch.
>

The way I've done it with legacy projects in the past is that you figure
out the number
of errors its currently getting, and fail the if that number increases.
 Whenever someone
checks in code which reduces the number, they can simultaneously reduce the
configured "max issues count" to the current number.


>
> Robin
>
> >
> >
> > On 6/10/12 10:17 PM, Robin Anil wrote:
> >
> >> Do it!
> >> On Jun 10, 2012 9:09 PM, "Benson Margulies"<bimargulies@gmail.**com<
> bimargulies@gmail.com>>
> >>  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<ro...@gmail.com>
> >>>  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<gsingers@apache.org
> >>>> 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
> >>>>>
> >>>>
> >
>



-- 

  -jake

Re: Turn on code inspections, please

[Robin Anil <ro...@gmail.com>]

------
Robin Anil


On Mon, Jun 11, 2012 at 9:30 AM, Jeff Eastman <jd...@windwardsolutions.com>wrote:

> +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.
>
> Does it have a threshold option? Like fail if issue count > 100 (or
whatever number is there at the moment) and strive to decrease that number
with each patch.

Robin

>
>
> On 6/10/12 10:17 PM, Robin Anil wrote:
>
>> Do it!
>> On Jun 10, 2012 9:09 PM, "Benson Margulies"<bi...@gmail.com>>
>>  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<ro...@gmail.com>
>>>  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<gsingers@apache.org
>>>> 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
>>>>>
>>>>
>

Re: Turn on code inspections, please

[Jeff Eastman <jd...@windwardsolutions.com>]

+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"<bi...@gmail.com>  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<ro...@gmail.com>  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<gsingers@apache.org
>>> 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

Re: Turn on code inspections, please

[Robin Anil <ro...@gmail.com>]

Do it!
On Jun 10, 2012 9:09 PM, "Benson Margulies" <bi...@gmail.com> 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 <ro...@gmail.com> 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 <gsingers@apache.org
> >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
>

Re: Turn on code inspections, please

[Ted Dunning <te...@gmail.com>]

If you mean eclipse integration with the style check, go ahead.  Something is better than nothing. 

Sent from my iPhone

On Jun 11, 2012, at 4:39 AM, Benson Margulies <bi...@gmail.com> wrote:

> It may take me a few days.
> 
> How much do people care about Eclipse integration? Can I ignore that
> for the moment?
> 
> On Sun, Jun 10, 2012 at 10:58 PM, Drew Farris <dr...@apache.org> wrote:
>> Benson, this sounds promising. Will you post a patch that turns these
>> on so we can take it out for a spin?
>> 
>> I think committers should not be checking in code that doesn't conform
>> to the project's standards.
>> 
>> Drew
>> 
>> On Sun, Jun 10, 2012 at 10:09 PM, Benson Margulies
>> <bi...@gmail.com> 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 <ro...@gmail.com> 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 <gs...@apache.org>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

Re: Turn on code inspections, please

[Dmitriy Lyubimov <dl...@gmail.com>]

both pmd and findbugs though seem to have eclipse and intellij plugins
though. I'll check them out.

On Mon, Jun 11, 2012 at 4:47 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
> "In practice, the rate of false warnings reported by FindBugs is
> generally less than 50%"
>
> which probably disqualifies it as a hard build stopper if 30-ish% is noise..
>
> On Mon, Jun 11, 2012 at 4:40 PM, Robin Anil <ro...@gmail.com> wrote:
>> pmd does have static analysis
>> http://maven.apache.org/plugins/maven-pmd-plugin/
>> use findbugs as well http://mojo.codehaus.org/findbugs-maven-plugin/
>> ------
>> Robin Anil
>>
>>
>> On Mon, Jun 11, 2012 at 6:36 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
>>
>>> Robin,
>>> what maven plugin does that? PMD?
>>>
>>> On Mon, Jun 11, 2012 at 3:52 PM, Robin Anil <ro...@gmail.com> wrote:
>>> > Can we just focus on the bugs first. Its more useful to get static
>>> analysis
>>> > in and make that a build blocker. Checkstyle is secondary and is a
>>> > sensitive topic.
>>> >
>>> > Robin
>>> >
>>> > On Mon, Jun 11, 2012 at 5:49 PM, Jeff Eastman <
>>> jdog@windwardsolutions.com>wrote:
>>> >
>>> >> +1 I feel the same way as Dmitriy
>>> >>
>>> >>
>>> >>
>>> >> On 6/11/12 6:39 PM, Dmitriy Lyubimov wrote:
>>> >>
>>> >>> which is why i am saying it would be easier to decouple code tools
>>> >>> from environment. That is, put them into the build process .
>>> >>>
>>> >>>  if we could relax some of the checks on top of it to degree they
>>> >>> actually agree with sun style autoformatter in eclipse, that would be
>>> >>> super-great (those are mostly things like spaces in comments and some
>>> >>> wrapping choices which are for some reason construed differently by
>>> >>> checkstyle and eclipse styles). I don't think there are many, if,
>>> >>> perhaps, any at any given checkstyle settings.
>>> >>>
>>> >>> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning<te...@gmail.com>
>>> >>>  wrote:
>>> >>>
>>> >>>> I don't want to ignite a flame war since (a) and (b) are sufficient
>>> >>>> grounds
>>> >>>> to stay with whatever you like to use, but
>>> >>>>
>>> >>>> c) I am not convinced of this
>>> >>>>
>>> >>>> d) IntelliJ has a free edition that works great.  You can also get the
>>> >>>> full
>>> >>>> version for free for use on open source projects.
>>> >>>>
>>> >>>> Like I said, this doesn't change your deep motor training.  I would
>>> point
>>> >>>> out that I have been through more generations of editing environments
>>> >>>> than
>>> >>>> I can count over the last 35 years and while it has always been
>>> >>>> unpleasant
>>> >>>> to change, changing to the next big thing has generally been
>>> worthwhile.
>>> >>>>
>>> >>>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>> >>>>  wrote:
>>> >>>>
>>> >>>>  And no, i am not ready to part with eclipse yet. a) years of reflexes
>>> >>>>> hard to beat. b) it is the only environment that has a lot of other
>>> >>>>> integrations that I use such as StatET (integrated R step-by-step
>>> >>>>> debugger&  help). c) faster native visuals. d) free.
>>> >>>>>
>>> >>>>>
>>> >>>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>> >>>>> wrote:
>>> >>>>>
>>> >>>>>> Yeah eclipse is a problem. Or checkstyle configuration. or both.
>>> >>>>>>
>>> >>>>>> I used to set up the checkstyle plugin that highlights the problems
>>> >>>>>> but i never managed to get it to 0 since it contradicts sun
>>> >>>>>> recommended formatting as set in eclipse styles (or even with
>>> >>>>>> adjustments). So both tools aimed at style actually contradict each
>>> >>>>>> other on what the right style is. Which tells me both are probably
>>> >>>>>> overzealous in their assertion since apparently those assertions
>>> are a
>>> >>>>>> subject of contention among authors of those tools.
>>> >>>>>>
>>> >>>>>> Eclipse concerns can be partially set aside if the maven build fails
>>> >>>>>> and reports why it fails so I don't have to check in and wait for
>>> >>>>>> Jenkins to shove it into my face.
>>> >>>>>>
>>> >>>>>> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>>> >>>>>> <jd...@windwardsolutions.com>  wrote:
>>> >>>>>>
>>> >>>>>>> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>> >>>>>>>
>>> >>>>>>>> It may take me a few days.
>>> >>>>>>>>
>>> >>>>>>>> How much do people care about Eclipse integration? Can I ignore
>>> that
>>> >>>>>>>> for the moment?
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>>  -1 No, I think Eclipse integration is important but I will devote
>>> >>>>>>> some
>>> >>>>>>> energy to helping here if I can
>>> >>>>>>>
>>> >>>>>>
>>> >>>
>>> >>
>>>

Re: Turn on code inspections, please

[Lance Norskog <go...@gmail.com>]

May I suggest finishing 0.7 and then doing a 0.7.1 that is only
style/pmd/findbug/etc. changes?

On Mon, Jun 11, 2012 at 6:06 PM, Drew Farris <dr...@apache.org> wrote:
> FWIW, It's my understanding that there is a fair amount of
> configuration that can be done with both pmd and findbugs that will
> let us tweak it to suit our needs.
>
> Of course someone has to >do< that configuration, but nevertheless it
> can be done.
>
> I think it would be a great first step to get pmd/findbugs/checkstyle
> integrated with our maven builds. This would at least give us some
> level enforcement pre-checkin and via jenkins.
>
> From my point of view, Eclipse integration is a nice to have, with the
> eventual goal of having the command-line tools, eclipse and intellij
> agreeing on what's broken and the right way to fix it. I'm strongly
> against >only< enforcing these things via an IDE, but I don't think
> anyone's suggesting this at this point.
>
> Full disclosure: as an Eclipse user, I've lived for years with
> half-baked maven eclipse integration, indigo being a worse offender
> than other. I sort of expect to bounce between the terminal and my ide
> for both builds and checkins to make sure everything is truly as it is
> when presented to me via the IDE.
>
> Drew
>
> On Mon, Jun 11, 2012 at 7:47 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
>> "In practice, the rate of false warnings reported by FindBugs is
>> generally less than 50%"
>>
>> which probably disqualifies it as a hard build stopper if 30-ish% is noise..
>>
>> On Mon, Jun 11, 2012 at 4:40 PM, Robin Anil <ro...@gmail.com> wrote:
>>> pmd does have static analysis
>>> http://maven.apache.org/plugins/maven-pmd-plugin/
>>> use findbugs as well http://mojo.codehaus.org/findbugs-maven-plugin/
>>> ------
>>> Robin Anil
>>>
>>>
>>> On Mon, Jun 11, 2012 at 6:36 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
>>>
>>>> Robin,
>>>> what maven plugin does that? PMD?
>>>>
>>>> On Mon, Jun 11, 2012 at 3:52 PM, Robin Anil <ro...@gmail.com> wrote:
>>>> > Can we just focus on the bugs first. Its more useful to get static
>>>> analysis
>>>> > in and make that a build blocker. Checkstyle is secondary and is a
>>>> > sensitive topic.
>>>> >
>>>> > Robin
>>>> >
>>>> > On Mon, Jun 11, 2012 at 5:49 PM, Jeff Eastman <
>>>> jdog@windwardsolutions.com>wrote:
>>>> >
>>>> >> +1 I feel the same way as Dmitriy
>>>> >>
>>>> >>
>>>> >>
>>>> >> On 6/11/12 6:39 PM, Dmitriy Lyubimov wrote:
>>>> >>
>>>> >>> which is why i am saying it would be easier to decouple code tools
>>>> >>> from environment. That is, put them into the build process .
>>>> >>>
>>>> >>>  if we could relax some of the checks on top of it to degree they
>>>> >>> actually agree with sun style autoformatter in eclipse, that would be
>>>> >>> super-great (those are mostly things like spaces in comments and some
>>>> >>> wrapping choices which are for some reason construed differently by
>>>> >>> checkstyle and eclipse styles). I don't think there are many, if,
>>>> >>> perhaps, any at any given checkstyle settings.
>>>> >>>
>>>> >>> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning<te...@gmail.com>
>>>> >>>  wrote:
>>>> >>>
>>>> >>>> I don't want to ignite a flame war since (a) and (b) are sufficient
>>>> >>>> grounds
>>>> >>>> to stay with whatever you like to use, but
>>>> >>>>
>>>> >>>> c) I am not convinced of this
>>>> >>>>
>>>> >>>> d) IntelliJ has a free edition that works great.  You can also get the
>>>> >>>> full
>>>> >>>> version for free for use on open source projects.
>>>> >>>>
>>>> >>>> Like I said, this doesn't change your deep motor training.  I would
>>>> point
>>>> >>>> out that I have been through more generations of editing environments
>>>> >>>> than
>>>> >>>> I can count over the last 35 years and while it has always been
>>>> >>>> unpleasant
>>>> >>>> to change, changing to the next big thing has generally been
>>>> worthwhile.
>>>> >>>>
>>>> >>>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>>> >>>>  wrote:
>>>> >>>>
>>>> >>>>  And no, i am not ready to part with eclipse yet. a) years of reflexes
>>>> >>>>> hard to beat. b) it is the only environment that has a lot of other
>>>> >>>>> integrations that I use such as StatET (integrated R step-by-step
>>>> >>>>> debugger&  help). c) faster native visuals. d) free.
>>>> >>>>>
>>>> >>>>>
>>>> >>>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>>> >>>>> wrote:
>>>> >>>>>
>>>> >>>>>> Yeah eclipse is a problem. Or checkstyle configuration. or both.
>>>> >>>>>>
>>>> >>>>>> I used to set up the checkstyle plugin that highlights the problems
>>>> >>>>>> but i never managed to get it to 0 since it contradicts sun
>>>> >>>>>> recommended formatting as set in eclipse styles (or even with
>>>> >>>>>> adjustments). So both tools aimed at style actually contradict each
>>>> >>>>>> other on what the right style is. Which tells me both are probably
>>>> >>>>>> overzealous in their assertion since apparently those assertions
>>>> are a
>>>> >>>>>> subject of contention among authors of those tools.
>>>> >>>>>>
>>>> >>>>>> Eclipse concerns can be partially set aside if the maven build fails
>>>> >>>>>> and reports why it fails so I don't have to check in and wait for
>>>> >>>>>> Jenkins to shove it into my face.
>>>> >>>>>>
>>>> >>>>>> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>>>> >>>>>> <jd...@windwardsolutions.com>  wrote:
>>>> >>>>>>
>>>> >>>>>>> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>>> >>>>>>>
>>>> >>>>>>>> It may take me a few days.
>>>> >>>>>>>>
>>>> >>>>>>>> How much do people care about Eclipse integration? Can I ignore
>>>> that
>>>> >>>>>>>> for the moment?
>>>> >>>>>>>>
>>>> >>>>>>>>
>>>> >>>>>>>>  -1 No, I think Eclipse integration is important but I will devote
>>>> >>>>>>> some
>>>> >>>>>>> energy to helping here if I can
>>>> >>>>>>>
>>>> >>>>>>
>>>> >>>
>>>> >>
>>>>



-- 
Lance Norskog
goksron@gmail.com

Re: Turn on code inspections, please

[Drew Farris <dr...@apache.org>]

FWIW, It's my understanding that there is a fair amount of
configuration that can be done with both pmd and findbugs that will
let us tweak it to suit our needs.

Of course someone has to >do< that configuration, but nevertheless it
can be done.

I think it would be a great first step to get pmd/findbugs/checkstyle
integrated with our maven builds. This would at least give us some
level enforcement pre-checkin and via jenkins.

>From my point of view, Eclipse integration is a nice to have, with the
eventual goal of having the command-line tools, eclipse and intellij
agreeing on what's broken and the right way to fix it. I'm strongly
against >only< enforcing these things via an IDE, but I don't think
anyone's suggesting this at this point.

Full disclosure: as an Eclipse user, I've lived for years with
half-baked maven eclipse integration, indigo being a worse offender
than other. I sort of expect to bounce between the terminal and my ide
for both builds and checkins to make sure everything is truly as it is
when presented to me via the IDE.

Drew

On Mon, Jun 11, 2012 at 7:47 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
> "In practice, the rate of false warnings reported by FindBugs is
> generally less than 50%"
>
> which probably disqualifies it as a hard build stopper if 30-ish% is noise..
>
> On Mon, Jun 11, 2012 at 4:40 PM, Robin Anil <ro...@gmail.com> wrote:
>> pmd does have static analysis
>> http://maven.apache.org/plugins/maven-pmd-plugin/
>> use findbugs as well http://mojo.codehaus.org/findbugs-maven-plugin/
>> ------
>> Robin Anil
>>
>>
>> On Mon, Jun 11, 2012 at 6:36 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
>>
>>> Robin,
>>> what maven plugin does that? PMD?
>>>
>>> On Mon, Jun 11, 2012 at 3:52 PM, Robin Anil <ro...@gmail.com> wrote:
>>> > Can we just focus on the bugs first. Its more useful to get static
>>> analysis
>>> > in and make that a build blocker. Checkstyle is secondary and is a
>>> > sensitive topic.
>>> >
>>> > Robin
>>> >
>>> > On Mon, Jun 11, 2012 at 5:49 PM, Jeff Eastman <
>>> jdog@windwardsolutions.com>wrote:
>>> >
>>> >> +1 I feel the same way as Dmitriy
>>> >>
>>> >>
>>> >>
>>> >> On 6/11/12 6:39 PM, Dmitriy Lyubimov wrote:
>>> >>
>>> >>> which is why i am saying it would be easier to decouple code tools
>>> >>> from environment. That is, put them into the build process .
>>> >>>
>>> >>>  if we could relax some of the checks on top of it to degree they
>>> >>> actually agree with sun style autoformatter in eclipse, that would be
>>> >>> super-great (those are mostly things like spaces in comments and some
>>> >>> wrapping choices which are for some reason construed differently by
>>> >>> checkstyle and eclipse styles). I don't think there are many, if,
>>> >>> perhaps, any at any given checkstyle settings.
>>> >>>
>>> >>> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning<te...@gmail.com>
>>> >>>  wrote:
>>> >>>
>>> >>>> I don't want to ignite a flame war since (a) and (b) are sufficient
>>> >>>> grounds
>>> >>>> to stay with whatever you like to use, but
>>> >>>>
>>> >>>> c) I am not convinced of this
>>> >>>>
>>> >>>> d) IntelliJ has a free edition that works great.  You can also get the
>>> >>>> full
>>> >>>> version for free for use on open source projects.
>>> >>>>
>>> >>>> Like I said, this doesn't change your deep motor training.  I would
>>> point
>>> >>>> out that I have been through more generations of editing environments
>>> >>>> than
>>> >>>> I can count over the last 35 years and while it has always been
>>> >>>> unpleasant
>>> >>>> to change, changing to the next big thing has generally been
>>> worthwhile.
>>> >>>>
>>> >>>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>> >>>>  wrote:
>>> >>>>
>>> >>>>  And no, i am not ready to part with eclipse yet. a) years of reflexes
>>> >>>>> hard to beat. b) it is the only environment that has a lot of other
>>> >>>>> integrations that I use such as StatET (integrated R step-by-step
>>> >>>>> debugger&  help). c) faster native visuals. d) free.
>>> >>>>>
>>> >>>>>
>>> >>>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>> >>>>> wrote:
>>> >>>>>
>>> >>>>>> Yeah eclipse is a problem. Or checkstyle configuration. or both.
>>> >>>>>>
>>> >>>>>> I used to set up the checkstyle plugin that highlights the problems
>>> >>>>>> but i never managed to get it to 0 since it contradicts sun
>>> >>>>>> recommended formatting as set in eclipse styles (or even with
>>> >>>>>> adjustments). So both tools aimed at style actually contradict each
>>> >>>>>> other on what the right style is. Which tells me both are probably
>>> >>>>>> overzealous in their assertion since apparently those assertions
>>> are a
>>> >>>>>> subject of contention among authors of those tools.
>>> >>>>>>
>>> >>>>>> Eclipse concerns can be partially set aside if the maven build fails
>>> >>>>>> and reports why it fails so I don't have to check in and wait for
>>> >>>>>> Jenkins to shove it into my face.
>>> >>>>>>
>>> >>>>>> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>>> >>>>>> <jd...@windwardsolutions.com>  wrote:
>>> >>>>>>
>>> >>>>>>> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>> >>>>>>>
>>> >>>>>>>> It may take me a few days.
>>> >>>>>>>>
>>> >>>>>>>> How much do people care about Eclipse integration? Can I ignore
>>> that
>>> >>>>>>>> for the moment?
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>>  -1 No, I think Eclipse integration is important but I will devote
>>> >>>>>>> some
>>> >>>>>>> energy to helping here if I can
>>> >>>>>>>
>>> >>>>>>
>>> >>>
>>> >>
>>>

Re: Turn on code inspections, please

[Dmitriy Lyubimov <dl...@gmail.com>]

"In practice, the rate of false warnings reported by FindBugs is
generally less than 50%"

which probably disqualifies it as a hard build stopper if 30-ish% is noise..

On Mon, Jun 11, 2012 at 4:40 PM, Robin Anil <ro...@gmail.com> wrote:
> pmd does have static analysis
> http://maven.apache.org/plugins/maven-pmd-plugin/
> use findbugs as well http://mojo.codehaus.org/findbugs-maven-plugin/
> ------
> Robin Anil
>
>
> On Mon, Jun 11, 2012 at 6:36 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
>
>> Robin,
>> what maven plugin does that? PMD?
>>
>> On Mon, Jun 11, 2012 at 3:52 PM, Robin Anil <ro...@gmail.com> wrote:
>> > Can we just focus on the bugs first. Its more useful to get static
>> analysis
>> > in and make that a build blocker. Checkstyle is secondary and is a
>> > sensitive topic.
>> >
>> > Robin
>> >
>> > On Mon, Jun 11, 2012 at 5:49 PM, Jeff Eastman <
>> jdog@windwardsolutions.com>wrote:
>> >
>> >> +1 I feel the same way as Dmitriy
>> >>
>> >>
>> >>
>> >> On 6/11/12 6:39 PM, Dmitriy Lyubimov wrote:
>> >>
>> >>> which is why i am saying it would be easier to decouple code tools
>> >>> from environment. That is, put them into the build process .
>> >>>
>> >>>  if we could relax some of the checks on top of it to degree they
>> >>> actually agree with sun style autoformatter in eclipse, that would be
>> >>> super-great (those are mostly things like spaces in comments and some
>> >>> wrapping choices which are for some reason construed differently by
>> >>> checkstyle and eclipse styles). I don't think there are many, if,
>> >>> perhaps, any at any given checkstyle settings.
>> >>>
>> >>> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning<te...@gmail.com>
>> >>>  wrote:
>> >>>
>> >>>> I don't want to ignite a flame war since (a) and (b) are sufficient
>> >>>> grounds
>> >>>> to stay with whatever you like to use, but
>> >>>>
>> >>>> c) I am not convinced of this
>> >>>>
>> >>>> d) IntelliJ has a free edition that works great.  You can also get the
>> >>>> full
>> >>>> version for free for use on open source projects.
>> >>>>
>> >>>> Like I said, this doesn't change your deep motor training.  I would
>> point
>> >>>> out that I have been through more generations of editing environments
>> >>>> than
>> >>>> I can count over the last 35 years and while it has always been
>> >>>> unpleasant
>> >>>> to change, changing to the next big thing has generally been
>> worthwhile.
>> >>>>
>> >>>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov<dl...@gmail.com>
>> >>>>  wrote:
>> >>>>
>> >>>>  And no, i am not ready to part with eclipse yet. a) years of reflexes
>> >>>>> hard to beat. b) it is the only environment that has a lot of other
>> >>>>> integrations that I use such as StatET (integrated R step-by-step
>> >>>>> debugger&  help). c) faster native visuals. d) free.
>> >>>>>
>> >>>>>
>> >>>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov<dl...@gmail.com>
>> >>>>> wrote:
>> >>>>>
>> >>>>>> Yeah eclipse is a problem. Or checkstyle configuration. or both.
>> >>>>>>
>> >>>>>> I used to set up the checkstyle plugin that highlights the problems
>> >>>>>> but i never managed to get it to 0 since it contradicts sun
>> >>>>>> recommended formatting as set in eclipse styles (or even with
>> >>>>>> adjustments). So both tools aimed at style actually contradict each
>> >>>>>> other on what the right style is. Which tells me both are probably
>> >>>>>> overzealous in their assertion since apparently those assertions
>> are a
>> >>>>>> subject of contention among authors of those tools.
>> >>>>>>
>> >>>>>> Eclipse concerns can be partially set aside if the maven build fails
>> >>>>>> and reports why it fails so I don't have to check in and wait for
>> >>>>>> Jenkins to shove it into my face.
>> >>>>>>
>> >>>>>> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>> >>>>>> <jd...@windwardsolutions.com>  wrote:
>> >>>>>>
>> >>>>>>> On 6/11/12 7:39 AM, Benson Margulies wrote:
>> >>>>>>>
>> >>>>>>>> It may take me a few days.
>> >>>>>>>>
>> >>>>>>>> How much do people care about Eclipse integration? Can I ignore
>> that
>> >>>>>>>> for the moment?
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>  -1 No, I think Eclipse integration is important but I will devote
>> >>>>>>> some
>> >>>>>>> energy to helping here if I can
>> >>>>>>>
>> >>>>>>
>> >>>
>> >>
>>

Re: Turn on code inspections, please

[Robin Anil <ro...@gmail.com>]

pmd does have static analysis
http://maven.apache.org/plugins/maven-pmd-plugin/
use findbugs as well http://mojo.codehaus.org/findbugs-maven-plugin/
------
Robin Anil


On Mon, Jun 11, 2012 at 6:36 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:

> Robin,
> what maven plugin does that? PMD?
>
> On Mon, Jun 11, 2012 at 3:52 PM, Robin Anil <ro...@gmail.com> wrote:
> > Can we just focus on the bugs first. Its more useful to get static
> analysis
> > in and make that a build blocker. Checkstyle is secondary and is a
> > sensitive topic.
> >
> > Robin
> >
> > On Mon, Jun 11, 2012 at 5:49 PM, Jeff Eastman <
> jdog@windwardsolutions.com>wrote:
> >
> >> +1 I feel the same way as Dmitriy
> >>
> >>
> >>
> >> On 6/11/12 6:39 PM, Dmitriy Lyubimov wrote:
> >>
> >>> which is why i am saying it would be easier to decouple code tools
> >>> from environment. That is, put them into the build process .
> >>>
> >>>  if we could relax some of the checks on top of it to degree they
> >>> actually agree with sun style autoformatter in eclipse, that would be
> >>> super-great (those are mostly things like spaces in comments and some
> >>> wrapping choices which are for some reason construed differently by
> >>> checkstyle and eclipse styles). I don't think there are many, if,
> >>> perhaps, any at any given checkstyle settings.
> >>>
> >>> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning<te...@gmail.com>
> >>>  wrote:
> >>>
> >>>> I don't want to ignite a flame war since (a) and (b) are sufficient
> >>>> grounds
> >>>> to stay with whatever you like to use, but
> >>>>
> >>>> c) I am not convinced of this
> >>>>
> >>>> d) IntelliJ has a free edition that works great.  You can also get the
> >>>> full
> >>>> version for free for use on open source projects.
> >>>>
> >>>> Like I said, this doesn't change your deep motor training.  I would
> point
> >>>> out that I have been through more generations of editing environments
> >>>> than
> >>>> I can count over the last 35 years and while it has always been
> >>>> unpleasant
> >>>> to change, changing to the next big thing has generally been
> worthwhile.
> >>>>
> >>>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov<dl...@gmail.com>
> >>>>  wrote:
> >>>>
> >>>>  And no, i am not ready to part with eclipse yet. a) years of reflexes
> >>>>> hard to beat. b) it is the only environment that has a lot of other
> >>>>> integrations that I use such as StatET (integrated R step-by-step
> >>>>> debugger&  help). c) faster native visuals. d) free.
> >>>>>
> >>>>>
> >>>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov<dl...@gmail.com>
> >>>>> wrote:
> >>>>>
> >>>>>> Yeah eclipse is a problem. Or checkstyle configuration. or both.
> >>>>>>
> >>>>>> I used to set up the checkstyle plugin that highlights the problems
> >>>>>> but i never managed to get it to 0 since it contradicts sun
> >>>>>> recommended formatting as set in eclipse styles (or even with
> >>>>>> adjustments). So both tools aimed at style actually contradict each
> >>>>>> other on what the right style is. Which tells me both are probably
> >>>>>> overzealous in their assertion since apparently those assertions
> are a
> >>>>>> subject of contention among authors of those tools.
> >>>>>>
> >>>>>> Eclipse concerns can be partially set aside if the maven build fails
> >>>>>> and reports why it fails so I don't have to check in and wait for
> >>>>>> Jenkins to shove it into my face.
> >>>>>>
> >>>>>> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
> >>>>>> <jd...@windwardsolutions.com>  wrote:
> >>>>>>
> >>>>>>> On 6/11/12 7:39 AM, Benson Margulies wrote:
> >>>>>>>
> >>>>>>>> It may take me a few days.
> >>>>>>>>
> >>>>>>>> How much do people care about Eclipse integration? Can I ignore
> that
> >>>>>>>> for the moment?
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>  -1 No, I think Eclipse integration is important but I will devote
> >>>>>>> some
> >>>>>>> energy to helping here if I can
> >>>>>>>
> >>>>>>
> >>>
> >>
>

Re: Turn on code inspections, please

[Dmitriy Lyubimov <dl...@gmail.com>]

Robin,
what maven plugin does that? PMD?

On Mon, Jun 11, 2012 at 3:52 PM, Robin Anil <ro...@gmail.com> wrote:
> Can we just focus on the bugs first. Its more useful to get static analysis
> in and make that a build blocker. Checkstyle is secondary and is a
> sensitive topic.
>
> Robin
>
> On Mon, Jun 11, 2012 at 5:49 PM, Jeff Eastman <jd...@windwardsolutions.com>wrote:
>
>> +1 I feel the same way as Dmitriy
>>
>>
>>
>> On 6/11/12 6:39 PM, Dmitriy Lyubimov wrote:
>>
>>> which is why i am saying it would be easier to decouple code tools
>>> from environment. That is, put them into the build process .
>>>
>>>  if we could relax some of the checks on top of it to degree they
>>> actually agree with sun style autoformatter in eclipse, that would be
>>> super-great (those are mostly things like spaces in comments and some
>>> wrapping choices which are for some reason construed differently by
>>> checkstyle and eclipse styles). I don't think there are many, if,
>>> perhaps, any at any given checkstyle settings.
>>>
>>> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning<te...@gmail.com>
>>>  wrote:
>>>
>>>> I don't want to ignite a flame war since (a) and (b) are sufficient
>>>> grounds
>>>> to stay with whatever you like to use, but
>>>>
>>>> c) I am not convinced of this
>>>>
>>>> d) IntelliJ has a free edition that works great.  You can also get the
>>>> full
>>>> version for free for use on open source projects.
>>>>
>>>> Like I said, this doesn't change your deep motor training.  I would point
>>>> out that I have been through more generations of editing environments
>>>> than
>>>> I can count over the last 35 years and while it has always been
>>>> unpleasant
>>>> to change, changing to the next big thing has generally been worthwhile.
>>>>
>>>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>>>  wrote:
>>>>
>>>>  And no, i am not ready to part with eclipse yet. a) years of reflexes
>>>>> hard to beat. b) it is the only environment that has a lot of other
>>>>> integrations that I use such as StatET (integrated R step-by-step
>>>>> debugger&  help). c) faster native visuals. d) free.
>>>>>
>>>>>
>>>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Yeah eclipse is a problem. Or checkstyle configuration. or both.
>>>>>>
>>>>>> I used to set up the checkstyle plugin that highlights the problems
>>>>>> but i never managed to get it to 0 since it contradicts sun
>>>>>> recommended formatting as set in eclipse styles (or even with
>>>>>> adjustments). So both tools aimed at style actually contradict each
>>>>>> other on what the right style is. Which tells me both are probably
>>>>>> overzealous in their assertion since apparently those assertions are a
>>>>>> subject of contention among authors of those tools.
>>>>>>
>>>>>> Eclipse concerns can be partially set aside if the maven build fails
>>>>>> and reports why it fails so I don't have to check in and wait for
>>>>>> Jenkins to shove it into my face.
>>>>>>
>>>>>> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>>>>>> <jd...@windwardsolutions.com>  wrote:
>>>>>>
>>>>>>> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>>>>>>
>>>>>>>> It may take me a few days.
>>>>>>>>
>>>>>>>> How much do people care about Eclipse integration? Can I ignore that
>>>>>>>> for the moment?
>>>>>>>>
>>>>>>>>
>>>>>>>>  -1 No, I think Eclipse integration is important but I will devote
>>>>>>> some
>>>>>>> energy to helping here if I can
>>>>>>>
>>>>>>
>>>
>>

Re: Turn on code inspections, please

[Robin Anil <ro...@gmail.com>]

Can we just focus on the bugs first. Its more useful to get static analysis
in and make that a build blocker. Checkstyle is secondary and is a
sensitive topic.

Robin

On Mon, Jun 11, 2012 at 5:49 PM, Jeff Eastman <jd...@windwardsolutions.com>wrote:

> +1 I feel the same way as Dmitriy
>
>
>
> On 6/11/12 6:39 PM, Dmitriy Lyubimov wrote:
>
>> which is why i am saying it would be easier to decouple code tools
>> from environment. That is, put them into the build process .
>>
>>  if we could relax some of the checks on top of it to degree they
>> actually agree with sun style autoformatter in eclipse, that would be
>> super-great (those are mostly things like spaces in comments and some
>> wrapping choices which are for some reason construed differently by
>> checkstyle and eclipse styles). I don't think there are many, if,
>> perhaps, any at any given checkstyle settings.
>>
>> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning<te...@gmail.com>
>>  wrote:
>>
>>> I don't want to ignite a flame war since (a) and (b) are sufficient
>>> grounds
>>> to stay with whatever you like to use, but
>>>
>>> c) I am not convinced of this
>>>
>>> d) IntelliJ has a free edition that works great.  You can also get the
>>> full
>>> version for free for use on open source projects.
>>>
>>> Like I said, this doesn't change your deep motor training.  I would point
>>> out that I have been through more generations of editing environments
>>> than
>>> I can count over the last 35 years and while it has always been
>>> unpleasant
>>> to change, changing to the next big thing has generally been worthwhile.
>>>
>>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>>  wrote:
>>>
>>>  And no, i am not ready to part with eclipse yet. a) years of reflexes
>>>> hard to beat. b) it is the only environment that has a lot of other
>>>> integrations that I use such as StatET (integrated R step-by-step
>>>> debugger&  help). c) faster native visuals. d) free.
>>>>
>>>>
>>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>>> wrote:
>>>>
>>>>> Yeah eclipse is a problem. Or checkstyle configuration. or both.
>>>>>
>>>>> I used to set up the checkstyle plugin that highlights the problems
>>>>> but i never managed to get it to 0 since it contradicts sun
>>>>> recommended formatting as set in eclipse styles (or even with
>>>>> adjustments). So both tools aimed at style actually contradict each
>>>>> other on what the right style is. Which tells me both are probably
>>>>> overzealous in their assertion since apparently those assertions are a
>>>>> subject of contention among authors of those tools.
>>>>>
>>>>> Eclipse concerns can be partially set aside if the maven build fails
>>>>> and reports why it fails so I don't have to check in and wait for
>>>>> Jenkins to shove it into my face.
>>>>>
>>>>> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>>>>> <jd...@windwardsolutions.com>  wrote:
>>>>>
>>>>>> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>>>>>
>>>>>>> It may take me a few days.
>>>>>>>
>>>>>>> How much do people care about Eclipse integration? Can I ignore that
>>>>>>> for the moment?
>>>>>>>
>>>>>>>
>>>>>>>  -1 No, I think Eclipse integration is important but I will devote
>>>>>> some
>>>>>> energy to helping here if I can
>>>>>>
>>>>>
>>
>

Re: Turn on code inspections, please

[Jeff Eastman <jd...@windwardsolutions.com>]

+1 I feel the same way as Dmitriy


On 6/11/12 6:39 PM, Dmitriy Lyubimov wrote:
> which is why i am saying it would be easier to decouple code tools
> from environment. That is, put them into the build process .
>
>   if we could relax some of the checks on top of it to degree they
> actually agree with sun style autoformatter in eclipse, that would be
> super-great (those are mostly things like spaces in comments and some
> wrapping choices which are for some reason construed differently by
> checkstyle and eclipse styles). I don't think there are many, if,
> perhaps, any at any given checkstyle settings.
>
> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning<te...@gmail.com>  wrote:
>> I don't want to ignite a flame war since (a) and (b) are sufficient grounds
>> to stay with whatever you like to use, but
>>
>> c) I am not convinced of this
>>
>> d) IntelliJ has a free edition that works great.  You can also get the full
>> version for free for use on open source projects.
>>
>> Like I said, this doesn't change your deep motor training.  I would point
>> out that I have been through more generations of editing environments than
>> I can count over the last 35 years and while it has always been unpleasant
>> to change, changing to the next big thing has generally been worthwhile.
>>
>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov<dl...@gmail.com>  wrote:
>>
>>> And no, i am not ready to part with eclipse yet. a) years of reflexes
>>> hard to beat. b) it is the only environment that has a lot of other
>>> integrations that I use such as StatET (integrated R step-by-step
>>> debugger&  help). c) faster native visuals. d) free.
>>>
>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>> wrote:
>>>> Yeah eclipse is a problem. Or checkstyle configuration. or both.
>>>>
>>>> I used to set up the checkstyle plugin that highlights the problems
>>>> but i never managed to get it to 0 since it contradicts sun
>>>> recommended formatting as set in eclipse styles (or even with
>>>> adjustments). So both tools aimed at style actually contradict each
>>>> other on what the right style is. Which tells me both are probably
>>>> overzealous in their assertion since apparently those assertions are a
>>>> subject of contention among authors of those tools.
>>>>
>>>> Eclipse concerns can be partially set aside if the maven build fails
>>>> and reports why it fails so I don't have to check in and wait for
>>>> Jenkins to shove it into my face.
>>>>
>>>> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>>>> <jd...@windwardsolutions.com>  wrote:
>>>>> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>>>>> It may take me a few days.
>>>>>>
>>>>>> How much do people care about Eclipse integration? Can I ignore that
>>>>>> for the moment?
>>>>>>
>>>>>>
>>>>> -1 No, I think Eclipse integration is important but I will devote some
>>>>> energy to helping here if I can
>

Re: Turn on code inspections, please

[Sean Owen <sr...@gmail.com>]

I attached my current inspection settings for IntelliJ 11 to the wiki at:

https://cwiki.apache.org/confluence/download/attachments/22872443/Sean.xml

It should show up as an inspection profile if you put it in the IntelilJ
prefs directory, under the "inspection" folder. On the Mac, find that at
~/Library/Preferences/IntelliJIdea11

On Tue, Jun 12, 2012 at 1:10 PM, Grant Ingersoll <gs...@apache.org>wrote:
>
> Is there a way in IntelliJ to publish what ones you have turned on?  It
> looks like in IntelliJ at least, one can publish their inspections.  Sean,
> perhaps you can upload yours to the Wiki and we can all start from that
> one, or at least the IntelliJ users can and someone can port it to Eclipse.

Re: Turn on code inspections, please

[Grant Ingersoll <gs...@apache.org>]

On Jun 11, 2012, at 11:37 PM, Sean Owen wrote:

> I myself am only advocating turning on *editor* support for avoiding even
> writing problems to begin with. I actually agree that post-commit hooks can
> be hard to use (error #39, stray space on line 319 of file X.java, now
> where is that...) and noisy. I don't want that burden.
> 
> I am merely surprised to see funny-looking Java code since, for Java, there
> has always been one standard across the industry from the start (Sun's),
> and with editor support, you kinda have to *try* to write such code. I do
> think tools matter in this regard; IntelliJ is more seamless in
> finding/fixing stuff but Eclipse does fine here too. Anything like
> Sun-style Java would be great; what's being committed is not nearly.

I've always used the Lucene one that we publish.  I think the only difference is it uses a tab/space size of 2 instead of 4.

We should probably just make sure we publish the Eclipse and IntelliJ ones we want people to use and make sure all committers use them.


> 
> 
> Style isn't important per se; it's a broken-windows thing. If nobody's
> bothered about keywords or C-style arrays, it's a good chance people aren't
> bothered about stuff static analysis can find. And that is correct here.
> Again -- I'm advocating things that just make these non-issues at time of
> writing. No hard work. It's hard to use the wrong variable in a loop when
> the IDE flags it, or says "this cast can't be right" or "'transient'
> doesn't make sense here" (Again: IntelliJ simply does this a mile better
> than Eclipse. I do think it shows when you use an IDE that flags this, and
> when you don't. FWIW -- IntelliJ dominated at Google.)

Is there a way in IntelliJ to publish what ones you have turned on?  It looks like in IntelliJ at least, one can publish their inspections.  Sean, perhaps you can upload yours to the Wiki and we can all start from that one, or at least the IntelliJ users can and someone can port it to Eclipse.


> 
> 
> Even this isn't as important per se as the real problems: the code just
> looks un-designed or at best differently designed in most every major
> package. Rotting code, deprecated APIs, incomplete stuff, TODO, a mile of
> copy-and-paste in Bayes, three different frameworks for the same thing
> (collections, Hadoop jobs, serialization).

I get the first ones, what's the issue on Bayes (was it the two different implementations?) and the 3 frameworks?  

> This is a real problem and tools
> won't detect this one. But when you aren't worried about formatting and
> small bugs, when you can refactor with tool support that much more easily
> -- you do. When it's hard you don't, and others don't. And that's been
> borne out here.
> 
> 
> I sense consensus that all this doesn't really matter so much, yes -- that
> is why it is this way indeed. My years of seeing bad software and good tell
> me that small-scale quality correlate with, and cause, big-scale quality.
> At this point I don't know that it is worth really 'fixing' Mahout; it will
> be superseded in a year or two anyway having accomplished some mission. But
> it still doesn't hurt to start chipping away, from the small stuff. I will
> stop tilting at this windmill since I'm not going to do new development in
> Mahout anyway.

Mea culpa on this one.  I think I used to be the big blocker on this, but I've come to see the light, so I'm all for us taking this on.

I do think there is still hope and it means we continue to focus more on our core capabilities and it likely starts with what the "broken windows" problem you are addressing here as well as the clean up and eviction of bad existing code instead of adding new code.  I think we've started on this lately, but obviously we need to continue this momentum for it to be real.

Re: Turn on code inspections, please

[Sean Owen <sr...@gmail.com>]

I myself am only advocating turning on *editor* support for avoiding even
writing problems to begin with. I actually agree that post-commit hooks can
be hard to use (error #39, stray space on line 319 of file X.java, now
where is that...) and noisy. I don't want that burden.

I am merely surprised to see funny-looking Java code since, for Java, there
has always been one standard across the industry from the start (Sun's),
and with editor support, you kinda have to *try* to write such code. I do
think tools matter in this regard; IntelliJ is more seamless in
finding/fixing stuff but Eclipse does fine here too. Anything like
Sun-style Java would be great; what's being committed is not nearly.


Style isn't important per se; it's a broken-windows thing. If nobody's
bothered about keywords or C-style arrays, it's a good chance people aren't
bothered about stuff static analysis can find. And that is correct here.
Again -- I'm advocating things that just make these non-issues at time of
writing. No hard work. It's hard to use the wrong variable in a loop when
the IDE flags it, or says "this cast can't be right" or "'transient'
doesn't make sense here" (Again: IntelliJ simply does this a mile better
than Eclipse. I do think it shows when you use an IDE that flags this, and
when you don't. FWIW -- IntelliJ dominated at Google.)


Even this isn't as important per se as the real problems: the code just
looks un-designed or at best differently designed in most every major
package. Rotting code, deprecated APIs, incomplete stuff, TODO, a mile of
copy-and-paste in Bayes, three different frameworks for the same thing
(collections, Hadoop jobs, serialization). This is a real problem and tools
won't detect this one. But when you aren't worried about formatting and
small bugs, when you can refactor with tool support that much more easily
-- you do. When it's hard you don't, and others don't. And that's been
borne out here.


I sense consensus that all this doesn't really matter so much, yes -- that
is why it is this way indeed. My years of seeing bad software and good tell
me that small-scale quality correlate with, and cause, big-scale quality.
At this point I don't know that it is worth really 'fixing' Mahout; it will
be superseded in a year or two anyway having accomplished some mission. But
it still doesn't hurt to start chipping away, from the small stuff. I will
stop tilting at this windmill since I'm not going to do new development in
Mahout anyway.


On Tue, Jun 12, 2012 at 12:07 AM, Jeff Eastman
<jd...@windwardsolutions.com>wrote:

> In my case the projects that actually generate revenue for me all use
> Eclipse so I have little incentive to switch horses at this point. If we
> are raising the bar on style so far that reasonable Eclipse tooling
> generates build failures then I am opposed to this level of style checking.
> I frankly think we are arguing about minutia and it is counterproductive.
>
>

Re: Turn on code inspections, please

[Jeff Eastman <jd...@windwardsolutions.com>]

In my case the projects that actually generate revenue for me all use 
Eclipse so I have little incentive to switch horses at this point. If we 
are raising the bar on style so far that reasonable Eclipse tooling 
generates build failures then I am opposed to this level of style 
checking. I frankly think we are arguing about minutia and it is 
counterproductive.

On 6/11/12 6:44 PM, Dmitriy Lyubimov wrote:
> note that both checkstyle plugin and eclipse styles are claiming to
> have "sun java" style settings yet they do not 100% agree with each
> other. Which makes grounds to argument that differences are not as
> fundamental if they cannot be interpreted the same way by different
> implementors claiming working off the same spec.
>
> On Mon, Jun 11, 2012 at 3:39 PM, Dmitriy Lyubimov<dl...@gmail.com>  wrote:
>> which is why i am saying it would be easier to decouple code tools
>> from environment. That is, put them into the build process .
>>
>>   if we could relax some of the checks on top of it to degree they
>> actually agree with sun style autoformatter in eclipse, that would be
>> super-great (those are mostly things like spaces in comments and some
>> wrapping choices which are for some reason construed differently by
>> checkstyle and eclipse styles). I don't think there are many, if,
>> perhaps, any at any given checkstyle settings.
>>
>> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning<te...@gmail.com>  wrote:
>>> I don't want to ignite a flame war since (a) and (b) are sufficient grounds
>>> to stay with whatever you like to use, but
>>>
>>> c) I am not convinced of this
>>>
>>> d) IntelliJ has a free edition that works great.  You can also get the full
>>> version for free for use on open source projects.
>>>
>>> Like I said, this doesn't change your deep motor training.  I would point
>>> out that I have been through more generations of editing environments than
>>> I can count over the last 35 years and while it has always been unpleasant
>>> to change, changing to the next big thing has generally been worthwhile.
>>>
>>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov<dl...@gmail.com>  wrote:
>>>
>>>> And no, i am not ready to part with eclipse yet. a) years of reflexes
>>>> hard to beat. b) it is the only environment that has a lot of other
>>>> integrations that I use such as StatET (integrated R step-by-step
>>>> debugger&  help). c) faster native visuals. d) free.
>>>>
>>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov<dl...@gmail.com>
>>>> wrote:
>>>>> Yeah eclipse is a problem. Or checkstyle configuration. or both.
>>>>>
>>>>> I used to set up the checkstyle plugin that highlights the problems
>>>>> but i never managed to get it to 0 since it contradicts sun
>>>>> recommended formatting as set in eclipse styles (or even with
>>>>> adjustments). So both tools aimed at style actually contradict each
>>>>> other on what the right style is. Which tells me both are probably
>>>>> overzealous in their assertion since apparently those assertions are a
>>>>> subject of contention among authors of those tools.
>>>>>
>>>>> Eclipse concerns can be partially set aside if the maven build fails
>>>>> and reports why it fails so I don't have to check in and wait for
>>>>> Jenkins to shove it into my face.
>>>>>
>>>>> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>>>>> <jd...@windwardsolutions.com>  wrote:
>>>>>> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>>>>>> It may take me a few days.
>>>>>>>
>>>>>>> How much do people care about Eclipse integration? Can I ignore that
>>>>>>> for the moment?
>>>>>>>
>>>>>>>
>>>>>> -1 No, I think Eclipse integration is important but I will devote some
>>>>>> energy to helping here if I can
>

Re: Turn on code inspections, please

[Dmitriy Lyubimov <dl...@gmail.com>]

note that both checkstyle plugin and eclipse styles are claiming to
have "sun java" style settings yet they do not 100% agree with each
other. Which makes grounds to argument that differences are not as
fundamental if they cannot be interpreted the same way by different
implementors claiming working off the same spec.

On Mon, Jun 11, 2012 at 3:39 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
> which is why i am saying it would be easier to decouple code tools
> from environment. That is, put them into the build process .
>
>  if we could relax some of the checks on top of it to degree they
> actually agree with sun style autoformatter in eclipse, that would be
> super-great (those are mostly things like spaces in comments and some
> wrapping choices which are for some reason construed differently by
> checkstyle and eclipse styles). I don't think there are many, if,
> perhaps, any at any given checkstyle settings.
>
> On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning <te...@gmail.com> wrote:
>> I don't want to ignite a flame war since (a) and (b) are sufficient grounds
>> to stay with whatever you like to use, but
>>
>> c) I am not convinced of this
>>
>> d) IntelliJ has a free edition that works great.  You can also get the full
>> version for free for use on open source projects.
>>
>> Like I said, this doesn't change your deep motor training.  I would point
>> out that I have been through more generations of editing environments than
>> I can count over the last 35 years and while it has always been unpleasant
>> to change, changing to the next big thing has generally been worthwhile.
>>
>> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
>>
>>> And no, i am not ready to part with eclipse yet. a) years of reflexes
>>> hard to beat. b) it is the only environment that has a lot of other
>>> integrations that I use such as StatET (integrated R step-by-step
>>> debugger & help). c) faster native visuals. d) free.
>>>
>>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov <dl...@gmail.com>
>>> wrote:
>>> > Yeah eclipse is a problem. Or checkstyle configuration. or both.
>>> >
>>> > I used to set up the checkstyle plugin that highlights the problems
>>> > but i never managed to get it to 0 since it contradicts sun
>>> > recommended formatting as set in eclipse styles (or even with
>>> > adjustments). So both tools aimed at style actually contradict each
>>> > other on what the right style is. Which tells me both are probably
>>> > overzealous in their assertion since apparently those assertions are a
>>> > subject of contention among authors of those tools.
>>> >
>>> > Eclipse concerns can be partially set aside if the maven build fails
>>> > and reports why it fails so I don't have to check in and wait for
>>> > Jenkins to shove it into my face.
>>> >
>>> > On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>>> > <jd...@windwardsolutions.com> wrote:
>>> >> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>> >>>
>>> >>> It may take me a few days.
>>> >>>
>>> >>> How much do people care about Eclipse integration? Can I ignore that
>>> >>> for the moment?
>>> >>>
>>> >>>
>>> >> -1 No, I think Eclipse integration is important but I will devote some
>>> >> energy to helping here if I can
>>>

Re: Turn on code inspections, please

[Dmitriy Lyubimov <dl...@gmail.com>]

which is why i am saying it would be easier to decouple code tools
from environment. That is, put them into the build process .

 if we could relax some of the checks on top of it to degree they
actually agree with sun style autoformatter in eclipse, that would be
super-great (those are mostly things like spaces in comments and some
wrapping choices which are for some reason construed differently by
checkstyle and eclipse styles). I don't think there are many, if,
perhaps, any at any given checkstyle settings.

On Mon, Jun 11, 2012 at 3:21 PM, Ted Dunning <te...@gmail.com> wrote:
> I don't want to ignite a flame war since (a) and (b) are sufficient grounds
> to stay with whatever you like to use, but
>
> c) I am not convinced of this
>
> d) IntelliJ has a free edition that works great.  You can also get the full
> version for free for use on open source projects.
>
> Like I said, this doesn't change your deep motor training.  I would point
> out that I have been through more generations of editing environments than
> I can count over the last 35 years and while it has always been unpleasant
> to change, changing to the next big thing has generally been worthwhile.
>
> On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
>
>> And no, i am not ready to part with eclipse yet. a) years of reflexes
>> hard to beat. b) it is the only environment that has a lot of other
>> integrations that I use such as StatET (integrated R step-by-step
>> debugger & help). c) faster native visuals. d) free.
>>
>> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov <dl...@gmail.com>
>> wrote:
>> > Yeah eclipse is a problem. Or checkstyle configuration. or both.
>> >
>> > I used to set up the checkstyle plugin that highlights the problems
>> > but i never managed to get it to 0 since it contradicts sun
>> > recommended formatting as set in eclipse styles (or even with
>> > adjustments). So both tools aimed at style actually contradict each
>> > other on what the right style is. Which tells me both are probably
>> > overzealous in their assertion since apparently those assertions are a
>> > subject of contention among authors of those tools.
>> >
>> > Eclipse concerns can be partially set aside if the maven build fails
>> > and reports why it fails so I don't have to check in and wait for
>> > Jenkins to shove it into my face.
>> >
>> > On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
>> > <jd...@windwardsolutions.com> wrote:
>> >> On 6/11/12 7:39 AM, Benson Margulies wrote:
>> >>>
>> >>> It may take me a few days.
>> >>>
>> >>> How much do people care about Eclipse integration? Can I ignore that
>> >>> for the moment?
>> >>>
>> >>>
>> >> -1 No, I think Eclipse integration is important but I will devote some
>> >> energy to helping here if I can
>>

Re: Turn on code inspections, please

[Ted Dunning <te...@gmail.com>]

I don't want to ignite a flame war since (a) and (b) are sufficient grounds
to stay with whatever you like to use, but

c) I am not convinced of this

d) IntelliJ has a free edition that works great.  You can also get the full
version for free for use on open source projects.

Like I said, this doesn't change your deep motor training.  I would point
out that I have been through more generations of editing environments than
I can count over the last 35 years and while it has always been unpleasant
to change, changing to the next big thing has generally been worthwhile.

On Mon, Jun 11, 2012 at 2:14 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:

> And no, i am not ready to part with eclipse yet. a) years of reflexes
> hard to beat. b) it is the only environment that has a lot of other
> integrations that I use such as StatET (integrated R step-by-step
> debugger & help). c) faster native visuals. d) free.
>
> On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov <dl...@gmail.com>
> wrote:
> > Yeah eclipse is a problem. Or checkstyle configuration. or both.
> >
> > I used to set up the checkstyle plugin that highlights the problems
> > but i never managed to get it to 0 since it contradicts sun
> > recommended formatting as set in eclipse styles (or even with
> > adjustments). So both tools aimed at style actually contradict each
> > other on what the right style is. Which tells me both are probably
> > overzealous in their assertion since apparently those assertions are a
> > subject of contention among authors of those tools.
> >
> > Eclipse concerns can be partially set aside if the maven build fails
> > and reports why it fails so I don't have to check in and wait for
> > Jenkins to shove it into my face.
> >
> > On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
> > <jd...@windwardsolutions.com> wrote:
> >> On 6/11/12 7:39 AM, Benson Margulies wrote:
> >>>
> >>> It may take me a few days.
> >>>
> >>> How much do people care about Eclipse integration? Can I ignore that
> >>> for the moment?
> >>>
> >>>
> >> -1 No, I think Eclipse integration is important but I will devote some
> >> energy to helping here if I can
>

Re: Turn on code inspections, please

[Dmitriy Lyubimov <dl...@gmail.com>]

And no, i am not ready to part with eclipse yet. a) years of reflexes
hard to beat. b) it is the only environment that has a lot of other
integrations that I use such as StatET (integrated R step-by-step
debugger & help). c) faster native visuals. d) free.

On Mon, Jun 11, 2012 at 2:09 PM, Dmitriy Lyubimov <dl...@gmail.com> wrote:
> Yeah eclipse is a problem. Or checkstyle configuration. or both.
>
> I used to set up the checkstyle plugin that highlights the problems
> but i never managed to get it to 0 since it contradicts sun
> recommended formatting as set in eclipse styles (or even with
> adjustments). So both tools aimed at style actually contradict each
> other on what the right style is. Which tells me both are probably
> overzealous in their assertion since apparently those assertions are a
> subject of contention among authors of those tools.
>
> Eclipse concerns can be partially set aside if the maven build fails
> and reports why it fails so I don't have to check in and wait for
> Jenkins to shove it into my face.
>
> On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
> <jd...@windwardsolutions.com> wrote:
>> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>>
>>> It may take me a few days.
>>>
>>> How much do people care about Eclipse integration? Can I ignore that
>>> for the moment?
>>>
>>>
>> -1 No, I think Eclipse integration is important but I will devote some
>> energy to helping here if I can

Re: Turn on code inspections, please

[Dmitriy Lyubimov <dl...@gmail.com>]

Yeah eclipse is a problem. Or checkstyle configuration. or both.

I used to set up the checkstyle plugin that highlights the problems
but i never managed to get it to 0 since it contradicts sun
recommended formatting as set in eclipse styles (or even with
adjustments). So both tools aimed at style actually contradict each
other on what the right style is. Which tells me both are probably
overzealous in their assertion since apparently those assertions are a
subject of contention among authors of those tools.

Eclipse concerns can be partially set aside if the maven build fails
and reports why it fails so I don't have to check in and wait for
Jenkins to shove it into my face.

On Mon, Jun 11, 2012 at 7:31 AM, Jeff Eastman
<jd...@windwardsolutions.com> wrote:
> On 6/11/12 7:39 AM, Benson Margulies wrote:
>>
>> It may take me a few days.
>>
>> How much do people care about Eclipse integration? Can I ignore that
>> for the moment?
>>
>>
> -1 No, I think Eclipse integration is important but I will devote some
> energy to helping here if I can

Re: Turn on code inspections, please

[Jeff Eastman <jd...@windwardsolutions.com>]

On 6/11/12 7:39 AM, Benson Margulies wrote:
> It may take me a few days.
>
> How much do people care about Eclipse integration? Can I ignore that
> for the moment?
>
>
-1 No, I think Eclipse integration is important but I will devote some 
energy to helping here if I can

Re: Turn on code inspections, please

[Benson Margulies <bi...@gmail.com>]

It may take me a few days.

How much do people care about Eclipse integration? Can I ignore that
for the moment?

On Sun, Jun 10, 2012 at 10:58 PM, Drew Farris <dr...@apache.org> wrote:
> Benson, this sounds promising. Will you post a patch that turns these
> on so we can take it out for a spin?
>
> I think committers should not be checking in code that doesn't conform
> to the project's standards.
>
> Drew
>
> On Sun, Jun 10, 2012 at 10:09 PM, Benson Margulies
> <bi...@gmail.com> 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 <ro...@gmail.com> 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 <gs...@apache.org>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

Re: Turn on code inspections, please

[Drew Farris <dr...@apache.org>]

Benson, this sounds promising. Will you post a patch that turns these
on so we can take it out for a spin?

I think committers should not be checking in code that doesn't conform
to the project's standards.

Drew

On Sun, Jun 10, 2012 at 10:09 PM, Benson Margulies
<bi...@gmail.com> 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 <ro...@gmail.com> 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 <gs...@apache.org>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

Re: Turn on code inspections, please

[Benson Margulies <bi...@gmail.com>]

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 <ro...@gmail.com> 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 <gs...@apache.org>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

Re: Turn on code inspections, please

[Grant Ingersoll <gs...@apache.org>]

On Jun 10, 2012, at 2:19 PM, Robin Anil 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.

Probably need a bit of both, but here's the issue https://issues.apache.org/jira/browse/MAHOUT-698

If anyone is at Hadoop Summit, perhaps we can hack on Mahout on Wednesday night.  That was really successful at Buzzwords, and might be a way for us to knock some of these things out.

-Grant

Re: Turn on code inspections, please

[Robin Anil <ro...@gmail.com>]

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 <gs...@apache.org>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

Re: Turn on code inspections, please

[Grant Ingersoll <gs...@apache.org>]

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