> On March 19, 2015, 3:58 a.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/message/MessageWriterTest.scala, lines 31-33 > > <https://reviews.apache.org/r/31742/diff/2/?file=896450#file896450line31> > > > > Could we just use Random.nextBytes()?
My intention was to have some control of the randomness. nextInt(10) restricts the byte values appear in the resulting array to (0,9). So it should compress well. > On March 19, 2015, 3:58 a.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/message/MessageWriterTest.scala, line 65 > > <https://reviews.apache.org/r/31742/diff/2/?file=896450#file896450line65> > > > > Could we explicitly import the implicits in where it's applied? That > > way it's clear where it's used. I agree in general. But this is "private" and a unit test. > On March 19, 2015, 3:58 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/message/MessageWriter.scala, lines 163-168 > > <https://reviews.apache.org/r/31742/diff/2/?file=896449#file896449line163> > > > > Perhaps it's clearer to write this as a do .. while? It feels a bit > > weird that the first statement is adding an amount that is initialized to 0. We have to initiallize amount to some value anyway even when we use "do..while". And I feel "while.." easier to read than "do..while" because "while" shows the termination condition upfront. > On March 19, 2015, 3:58 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/message/MessageWriter.scala, line 143 > > <https://reviews.apache.org/r/31742/diff/2/?file=896449#file896449line143> > > > > Perhaps we can rename off to initOffset and offset to currOffset? This is consistent with OutputStream's parameter naming. - Yasuhiro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31742/#review76985 ----------------------------------------------------------- On March 16, 2015, 10:19 p.m., Yasuhiro Matsuda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31742/ > ----------------------------------------------------------- > > (Updated March 16, 2015, 10:19 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-527 > https://issues.apache.org/jira/browse/KAFKA-527 > > > Repository: kafka > > > Description > ------- > > less byte copies > > > Diffs > ----- > > core/src/main/scala/kafka/message/ByteBufferMessageSet.scala > 9c694719dc9b515fb3c3ae96435a87b334044272 > core/src/main/scala/kafka/message/MessageWriter.scala PRE-CREATION > core/src/test/scala/unit/kafka/message/MessageWriterTest.scala PRE-CREATION > > Diff: https://reviews.apache.org/r/31742/diff/ > > > Testing > ------- > > > Thanks, > > Yasuhiro Matsuda > >
