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


Thanks for the patch Brock. Some comments follow:

* Please remove any trailing whitespaces from the code where present. These are 
highlighted in red on the review board.


flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java
<https://reviews.apache.org/r/3928/#comment11704>

    Need to guard against a null body.



flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java
<https://reviews.apache.org/r/3928/#comment11706>

    This is the same variable name as what is used on line 80. Please use a 
different name for avoiding confusion.
    
    Also, ideally, the character encoding should be read from a system header 
value within the event. If the header is not present, the default encoding of 
UTF-8 could be used. That way the representation stays flexible.



flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java
<https://reviews.apache.org/r/3928/#comment11705>

    would be good to log this exception as well.


- Arvind


On 2012-02-17 19:51:58, Brock Noland wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3928/
> -----------------------------------------------------------
> 
> (Updated 2012-02-17 19:51:58)
> 
> 
> Review request for Flume.
> 
> 
> Summary
> -------
> 
> Changes SimpleEvent.toString() to include a human readable representation of 
> the body byte array.
> 
> 
> This addresses bug FLUME-828.
>     https://issues.apache.org/jira/browse/FLUME-828
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java e0c3b45 
>   flume-ng-core/src/test/java/org/apache/flume/event/TestSimpleEvent.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/3928/diff
> 
> 
> Testing
> -------
> 
> Added two tests for the new code.
> 
> 
> Thanks,
> 
> Brock
> 
>

Reply via email to