Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Content-Type: multipart/alternative;
 boundary="===============1827545040571985321=="
MIME-Version: 1.0
Subject: Re: Review Request 25260: Throw exception on metadata constraint
 violation, instead of retrying
From: "Christopher Tubbs" <ctubbsii-rb@apache.org>
To: "accumulo" <dev@accumulo.apache.org>,
 "Christopher Tubbs" <ctubbsii-rb@apache.org>,
 "Josh Elser" <josh.elser@gmail.com>, keith@deenlo.com,
 "Mike Drob" <mdrob@mdrob.com>
Date: Tue, 02 Sep 2014 23:06:41 -0000
Message-ID: <20140902230641.16960.45721@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Christopher Tubbs" <noreply@reviews.apache.org>
References: <20140902223242.16961.18092@reviews.apache.org>
In-Reply-To: <20140902223242.16961.18092@reviews.apache.org>
Reply-To: "Christopher Tubbs" <ctubbsii-rb@apache.org>

--===============1827545040571985321==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Sept. 2, 2014, 6: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.
> 
> Josh Elser wrote:
>     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).

One can't assume that a person inspecting the released code has access to the history or the JIRA. Those are nice supplemental resources, but understanding what is being tested, at a basic level, should not depend on them. A meaningful test name gives some scope to the issue in log output without a lookup, regardless of access to history or JIRA, so I think the name change suggestion is a good idea. Further, Josh's suggestion to bump the JIRA number to a comment I think also is a good idea, because it provides additional linkage to the JIRA in the absence of the git history.


- Christopher


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


On Sept. 2, 2014, 1: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, 1: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
> 
>


--===============1827545040571985321==--