accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Busbey" <s...@manvsbeard.com>
Subject Re: Review Request 15857: ACCUMULO-1931 - Javadoc for core.data
Date Wed, 27 Nov 2013 08:43:41 GMT

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



core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java
<https://reviews.apache.org/r/15857/#comment56714>

    s/invalide/invalid
    
    Actually "out of bounds for the given byte array" would be clearer.



core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java
<https://reviews.apache.org/r/15857/#comment56715>

    This is not quite correct. If there isn't a backing array, the function does a relative
bulk get for all remaining bytes in the buffer.
    
    It won't get bytes in the buffer from before the current position, nor after the current
limit.



core/src/main/java/org/apache/accumulo/core/data/Column.java
<https://reviews.apache.org/r/15857/#comment56716>

    worth noting that the member is passed back directly and is thus modifiable by the caller?



core/src/main/java/org/apache/accumulo/core/data/Column.java
<https://reviews.apache.org/r/15857/#comment56717>

    worth noting that the member is passed back directly and is thus modifiable by the caller?



core/src/main/java/org/apache/accumulo/core/data/Column.java
<https://reviews.apache.org/r/15857/#comment56718>

    worth noting that the member is passed back directly and is thus modifiable by the caller?



core/src/main/java/org/apache/accumulo/core/data/Column.java
<https://reviews.apache.org/r/15857/#comment56719>

    and nulls are treated as empty strings.



core/src/main/java/org/apache/accumulo/core/data/ColumnUpdate.java
<https://reviews.apache.org/r/15857/#comment56720>

    worth noting that the member is passed back directly and is thus modifiable by the caller?



core/src/main/java/org/apache/accumulo/core/data/ColumnUpdate.java
<https://reviews.apache.org/r/15857/#comment56721>

    worth noting that the member is passed back directly and is thus modifiable by the caller?



core/src/main/java/org/apache/accumulo/core/data/Condition.java
<https://reviews.apache.org/r/15857/#comment56725>

    visibility is set to an empty byte array, not null.



core/src/main/java/org/apache/accumulo/core/data/Condition.java
<https://reviews.apache.org/r/15857/#comment56726>

    visibility is set to an empty byte array, not null.



core/src/main/java/org/apache/accumulo/core/data/Condition.java
<https://reviews.apache.org/r/15857/#comment56727>

    visibility is set to an empty byte array, not null.



core/src/main/java/org/apache/accumulo/core/data/Condition.java
<https://reviews.apache.org/r/15857/#comment56728>

    visibility is set to an empty byte array, not null.



core/src/main/java/org/apache/accumulo/core/data/Condition.java
<https://reviews.apache.org/r/15857/#comment56729>

    s/Inorder/In order



core/src/main/java/org/apache/accumulo/core/data/Condition.java
<https://reviews.apache.org/r/15857/#comment56730>

    s/Inorder/In order



core/src/main/java/org/apache/accumulo/core/data/Condition.java
<https://reviews.apache.org/r/15857/#comment56731>

    s/Inorder/In order



core/src/main/java/org/apache/accumulo/core/data/Condition.java
<https://reviews.apache.org/r/15857/#comment56732>

    s/Inorder/In order



core/src/main/java/org/apache/accumulo/core/data/Key.java
<https://reviews.apache.org/r/15857/#comment56733>

    worth pointing out that it's printable when considering the byte array as ASCII.



core/src/main/java/org/apache/accumulo/core/data/Key.java
<https://reviews.apache.org/r/15857/#comment56734>

    worth pointing out that it's printable when considering the byte array as ASCII.



core/src/main/java/org/apache/accumulo/core/data/Key.java
<https://reviews.apache.org/r/15857/#comment56736>

    worth pointing out that it's turning it into a string when considering the bytes as ASCII.
    
    also a pointer to the caveats on appendPrintableString?



core/src/main/java/org/apache/accumulo/core/data/Key.java
<https://reviews.apache.org/r/15857/#comment56735>

    s/coliumn/column



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56761>

    Worth noting that a delete marker will hide any entries for that row column with an earlier
timestamp? And that Accumulo will eventually remove said data from disk as a part of normal
garbage collection?



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56737>

    Any chance for @since markers on these versions, or a pointer here to their details /
differences?



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56738>

    note that the buffer will be copied?



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56739>

    note that the buffer will be copied?



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56740>

    note that the Text contents will be copied?



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56741>

    will have an empty visibility string.
    
    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56743>

    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56742>

    will have an empty visibility string



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56744>

    will only match an empty visibility string.
    
    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56745>

    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56746>

    will only match an empty visibility string.
    



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56747>

    will have an empty visibility string.
    
    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56748>

    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56749>

    will have an empty visibility string.



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56750>

    will only match an empty visibility string.
    
    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56751>

    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56752>

    will only match an empty visibility string.
    



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56753>

    will have an empty visibility string.
    
    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56754>

    will have an empty visibility string.



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56755>

    will have an empty visibility string.
    
    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56756>

    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56757>

    will have an empty visibility string.



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56758>

    will only match an empty visibility string.
    
    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56759>

    will have a timestamp set when received by a tablet server



core/src/main/java/org/apache/accumulo/core/data/Mutation.java
<https://reviews.apache.org/r/15857/#comment56760>

    will have an empty visibility string.



core/src/main/java/org/apache/accumulo/core/data/Range.java
<https://reviews.apache.org/r/15857/#comment56762>

    Are we sure this constructor purposefully allows the end key to be before the start key?
    
    including it in the javadocs here makes it expected behavior for the future, rather than
a potential bug.



core/src/main/java/org/apache/accumulo/core/data/Range.java
<https://reviews.apache.org/r/15857/#comment56763>

    this method can optionally return null if the ranges do not overlap, instead of throwing
an exception. The returnNullIfDisjoint parameter controls this behavior.



core/src/main/java/org/apache/accumulo/core/data/Value.java
<https://reviews.apache.org/r/15857/#comment56764>

    Include a note about copying or not?



core/src/main/java/org/apache/accumulo/core/data/Value.java
<https://reviews.apache.org/r/15857/#comment56765>

    Maybe the above should say the copy flag can indicate that the caller wants to force a
copy?
    
    then this comment could be a TODO to indicate that toBytes means we always copy at least
once, and twice when copy is true?



core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java
<https://reviews.apache.org/r/15857/#comment56766>

    Should note what the variable-length encoding is. AFAICT, this is just a reimplementaiton
of o.a.h.io.WritableUtils.writeVInt to work with the underlying buffer.
    
    Could copy/paste description from there or link to it.



core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java
<https://reviews.apache.org/r/15857/#comment56767>

    Should note what the variable-length encoding is. AFAICT, this is just a reimplementaiton
of o.a.h.io.WritableUtils.writeVLong to work with the underlying buffer.
    
    Could copy/paste description from there or link to it.


- Sean Busbey


On Nov. 26, 2013, 6:13 p.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15857/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2013, 6:13 p.m.)
> 
> 
> Review request for accumulo.
> 
> 
> Bugs: ACCUMULO-1931
>     https://issues.apache.org/jira/browse/ACCUMULO-1931
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Javadoc additions and updates to classes in org.apache.accumulo.core.data.
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java eaa61b9950b691ed5955af3178a3aa939291e8bf

>   core/src/main/java/org/apache/accumulo/core/data/ByteSequence.java 4dc921c5efc44d374032c623f50e4218aa6c7299

>   core/src/main/java/org/apache/accumulo/core/data/Column.java a56c01d6604a30153739ed1329d034e3932f2e2f

>   core/src/main/java/org/apache/accumulo/core/data/ColumnUpdate.java 641ca3a740ed384ab93f4b2d3c08c928daadfb2f

>   core/src/main/java/org/apache/accumulo/core/data/ComparableBytes.java ce61844375617c36ede5e0e411b439205329b023

>   core/src/main/java/org/apache/accumulo/core/data/Condition.java fc8f2bf0394ae658bacfa94804b4b81f19a0c0c2

>   core/src/main/java/org/apache/accumulo/core/data/ConstraintViolationSummary.java 2929bc610df9352374621d29caea7732bd50847d

>   core/src/main/java/org/apache/accumulo/core/data/Key.java de9e22d9ebff5442633dc578613fd7d033afc9c5

>   core/src/main/java/org/apache/accumulo/core/data/KeyValue.java cc48322f9785ad508cc7925279f5e559629f3721

>   core/src/main/java/org/apache/accumulo/core/data/Mutation.java 5e281f2bfef7e164c9f0e0c9a7132e94012d7186

>   core/src/main/java/org/apache/accumulo/core/data/PartialKey.java 2049881fe26b5f237db39a412a88e2e2017f5cc9

>   core/src/main/java/org/apache/accumulo/core/data/Range.java 70857348cefd0941e5ffcf6e5d569f09870101e4

>   core/src/main/java/org/apache/accumulo/core/data/Value.java 7d3cf8f20aa12205d25027d6ad06e4a3f6062b2e

>   core/src/main/java/org/apache/accumulo/core/util/UnsynchronizedBuffer.java b640581bebbb87f1ac9a36f71275fa5dc8ef1223

> 
> Diff: https://reviews.apache.org/r/15857/diff/
> 
> 
> Testing
> -------
> 
> Compiled successfully; mvn javadoc:javadoc ran with no errors from the javadoc in this
patch.
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


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