accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Josh Elser" <josh.el...@gmail.com>
Subject Re: Review Request 25260: Throw exception on metadata constraint violation, instead of retrying
Date Tue, 02 Sep 2014 22:53:27 GMT


> On Sept. 2, 2014, 10:32 p.m., Josh Elser wrote:
> > test/src/test/java/org/apache/accumulo/test/Accumulo3096IT.java, line 34
> > <https://reviews.apache.org/r/25260/diff/1/?file=674070#file674070line34>
> >
> >     It'd be much nicer to have a test name that is more meaningful than a ticket.
Having the ticket referenced in a comment is useful if I want to find out more, but at first
glance it doesn't tell me anything about what it's testing.
> 
> Mike Drob wrote:
>     Why is it even useful as a comment? It should already be in the commit message and
'git blame' will tell you who and where it came from.

That's valid -- a bit more personal preference, I suppose. I prefer to have information in
the code rather than have to look at the history to find when it was introduced, especially
when the most recent change isn't from the ticket that originally introduced it (when blame
would be wrong).


- Josh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25260/#review52094
-----------------------------------------------------------


On Sept. 2, 2014, 5:45 p.m., kturner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25260/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2014, 5:45 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-3096
>     https://issues.apache.org/jira/browse/ACCUMULO-3096
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Throw exception on metadata constraint violation, instead of retrying
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/client/impl/Writer.java d6762e7 
>   server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java 463ca57

>   test/src/test/java/org/apache/accumulo/test/Accumulo3096IT.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25260/diff/
> 
> 
> Testing
> -------
> 
> Ran mvn package w/o incident .   Currently running mvn verify.
> 
> 
> Thanks,
> 
> kturner
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message