[jira] Commented: (MAHOUT-10) Replace fall-through exception handlers with propagated unchecked exception.

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (MAHOUT-10) Replace fall-through exception handlers with propagated unchecked exception.

ASF GitHub Bot (Jira)

    [ https://issues.apache.org/jira/browse/MAHOUT-10?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12576749#action_12576749 ]

Dawid Weiss commented on MAHOUT-10:
-----------------------------------

Hey guys. What's our committing policy? Can I commit issues nodoby objected to for some time? Especially if they are trivial, like this one?

> Replace fall-through exception handlers with propagated unchecked exception.
> ----------------------------------------------------------------------------
>
>                 Key: MAHOUT-10
>                 URL: https://issues.apache.org/jira/browse/MAHOUT-10
>             Project: Mahout
>          Issue Type: Improvement
>          Components: Clustering
>    Affects Versions: 0.1
>            Reporter: Dawid Weiss
>            Assignee: Dawid Weiss
>            Priority: Minor
>         Attachments: mah-10.patch
>
>
> I am doing a belated code review. There certain issues that I would like to change, for example fall-through exception handlers like this one:
>     try {
>       Class cl = Class.forName(job.get(DISTANCE_MEASURE_KEY));
>       measure = (DistanceMeasure) cl.newInstance();
>       measure.configure(job);
>     } catch (Exception e) {
>       e.printStackTrace();
>     }
> This prints the stack trace of an exception to the console, but continues thread's execution after the catch clause. Since distance measure key is required, this makes little sense. A runtime exception should be thrown -- this stops the job and causes a full stack trace to be displayed anyway (with the nested exception's message).

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

Committing was Re: [jira] Commented: (MAHOUT-10) Replace fall-through exception handlers with propagated unchecked exception.

Grant Ingersoll-2
Definitely.  You have full powers :-)

General rule of thumb is to use your judgment.  If it is a significant  
change, then ask around and/or say something like "I plan on  
committing in 2 days or something like that."  Minor changes can be  
committed sooner.  Both major and minor should have a JIRA issue  
associated.  Typos and small doc changes don't, IMO.

I usually do my commit messages as something like "MAHOUT-X: blah,  
blah, blah".  If you use this format, then JIRA can automatically  
extract the revision from SVN and display it under the Subversion  
section of that issue.  Then, when I commit, I also note, when  
resolving the issue, what revision I committed on.

-Grant


On Mar 9, 2008, at 7:09 AM, Dawid Weiss (JIRA) wrote:

>
>    [ https://issues.apache.org/jira/browse/MAHOUT-10?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12576749 
> #action_12576749 ]
>
> Dawid Weiss commented on MAHOUT-10:
> -----------------------------------
>
> Hey guys. What's our committing policy? Can I commit issues nodoby  
> objected to for some time? Especially if they are trivial, like this  
> one?
>
>> Replace fall-through exception handlers with propagated unchecked  
>> exception.
>> ----------------------------------------------------------------------------
>>
>>                Key: MAHOUT-10
>>                URL: https://issues.apache.org/jira/browse/MAHOUT-10
>>            Project: Mahout
>>         Issue Type: Improvement
>>         Components: Clustering
>>   Affects Versions: 0.1
>>           Reporter: Dawid Weiss
>>           Assignee: Dawid Weiss
>>           Priority: Minor
>>        Attachments: mah-10.patch
>>
>>
>> I am doing a belated code review. There certain issues that I would  
>> like to change, for example fall-through exception handlers like  
>> this one:
>>    try {
>>      Class cl = Class.forName(job.get(DISTANCE_MEASURE_KEY));
>>      measure = (DistanceMeasure) cl.newInstance();
>>      measure.configure(job);
>>    } catch (Exception e) {
>>      e.printStackTrace();
>>    }
>> This prints the stack trace of an exception to the console, but  
>> continues thread's execution after the catch clause. Since distance  
>> measure key is required, this makes little sense. A runtime  
>> exception should be thrown -- this stops the job and causes a full  
>> stack trace to be displayed anyway (with the nested exception's  
>> message).
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>

--------------------------
Grant Ingersoll
http://www.lucenebootcamp.com
Next Training: April 7, 2008 at ApacheCon Europe in Amsterdam

Lucene Helpful Hints:
http://wiki.apache.org/lucene-java/BasicsOfPerformance
http://wiki.apache.org/lucene-java/LuceneFAQ





Reply | Threaded
Open this post in threaded view
|

Re: Committing was Re: [jira] Commented: (MAHOUT-10) Replace fall-through exception handlers with propagated unchecked exception.

Dawid Weiss

> Definitely.  You have full powers :-)

I know. I committed all pending stuff -- one with some problems so you'll see
multiple commits for a single issue (my fault, won't happen again).

> change, then ask around and/or say something like "I plan on committing
> in 2 days or something like that."  Minor changes can be committed

Sounds good to me.

> I usually do my commit messages as something like "MAHOUT-X: blah, blah,

I usually did it in the other way "blah blah blah, MAHOUT-X". Good to know it's
the opposite.

D.
Reply | Threaded
Open this post in threaded view
|

Re: Committing was Re: [jira] Commented: (MAHOUT-10) Replace fall-through exception handlers with propagated unchecked exception.

Grant Ingersoll-2

On Mar 9, 2008, at 9:52 AM, Dawid Weiss wrote:

>
>> Definitely.  You have full powers :-)
>
> I know. I committed all pending stuff -- one with some problems so  
> you'll see multiple commits for a single issue (my fault, won't  
> happen again).
>


It will happen again, trust me :-)  I wouldn't worry about it,  
though.  Especially, in pre-1.x land.  Every committer I've ever known  
has done it more than once.  The more important thing is that you  
caught it and fixed it.  Even falling short of that, someone will  
catch it sooner or later.
Reply | Threaded
Open this post in threaded view
|

Re: Committing was Re: [jira] Commented: (MAHOUT-10) Replace fall-through exception handlers with propagated unchecked exception.

Dawid Weiss

> It will happen again, trust me :-)  I wouldn't worry about it, though.  

Yeah, well, this was, hmm... let's call it Sunday negligence ;) I didn't think
the point patch could affect anything else, so I failed to run unit tests before
the commit; it turned out canopy clustering used some very specific test conditions.

D.

> Especially, in pre-1.x land.  Every committer I've ever known has done
> it more than once.  The more important thing is that you caught it and
> fixed it.  Even falling short of that, someone will catch it sooner or
> later.
Reply | Threaded
Open this post in threaded view
|

Re: Committing was Re: [jira] Commented: (MAHOUT-10) Replace fall-through exception handlers with propagated unchecked exception.

Isabel Drost-3
In reply to this post by Grant Ingersoll-2
On Sunday 09 March 2008, Grant Ingersoll wrote:
> General rule of thumb is to use your judgment.  If it is a significant
> change, then ask around and/or say something like "I plan on
> committing in 2 days or something like that."  Minor changes can be
> committed sooner.

Sounds good to me as well.

Isabel  

--
You are wise, witty, and wonderful, but you spend too much time reading this
sort of trash.
  |\      _,,,---,,_       Web:   <http://www.isabel-drost.de>
  /,`.-'`'    -.  ;-;;,_
 |,4-  ) )-,_..;\ (  `'-'
'---''(_/--'  `-'\_) (fL)  IM:  <xmpp://[hidden email]>

signature.asc (196 bytes) Download Attachment