> 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.
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. > On Sept. 2, 2014, 10:32 p.m., Josh Elser wrote: > > server/base/src/main/java/org/apache/accumulo/server/util/MetadataTableUtil.java, > > line 149 > > <https://reviews.apache.org/r/25260/diff/1/?file=674069#file674069line149> > > > > A comment about why we throw a RTE for ConstraintViolationException but > > none of the others would be nice (in other words, reference the reason > > these changes are being done). HBase has a DoNotRetryIOException, maybe something like that would be interesting to consider. - Mike ----------------------------------------------------------- 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 > >
