> On 2012-02-28 01:06:14, Arvind Prabhakar wrote: > > 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.
Sorry about that, created this patch before I saw the policy. > On 2012-02-28 01:06:14, Arvind Prabhakar wrote: > > flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java, line 66 > > <https://reviews.apache.org/r/3928/diff/2/?file=75703#file75703line66> > > > > Need to guard against a null body. Fixed > On 2012-02-28 01:06:14, Arvind Prabhakar wrote: > > flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java, line 78 > > <https://reviews.apache.org/r/3928/diff/2/?file=75703#file75703line78> > > > > would be good to log this exception as well. Fixed > On 2012-02-28 01:06:14, Arvind Prabhakar wrote: > > flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java, line 71 > > <https://reviews.apache.org/r/3928/diff/2/?file=75703#file75703line71> > > > > 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. This String is coming from Hexdump.dump. I looked and it does not specify a character set while building the string so it will get the default. I removed the charset so we will get the same default. - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3928/#review5375 ----------------------------------------------------------- On 2012-02-28 13:46:28, Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3928/ > ----------------------------------------------------------- > > (Updated 2012-02-28 13:46:28) > > > 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/test/java/org/apache/flume/event/TestSimpleEvent.java > PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java e0c3b45 > > Diff: https://reviews.apache.org/r/3928/diff > > > Testing > ------- > > Added two tests for the new code. > > > Thanks, > > Brock > >
