----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18606/#review35859 -----------------------------------------------------------
Ship it! Overall looks good. Would be helpful to have some //comments in the gradle files. Regarding TestUtil, I think it might be better to open a separate ticket to clean it up. build.gradle <https://reviews.apache.org/r/18606/#comment66641> Can you add a comment here to describe what this is doing? gradle/dependency-versions-scala-2.10.gradle <https://reviews.apache.org/r/18606/#comment66642> Can you add a comment here to describe what this is doing? gradle/license.gradle <https://reviews.apache.org/r/18606/#comment66644> Can you add a comment here to describe what this is doing? samza-core/src/test/scala/org/apache/samza/util/TestUtil.scala <https://reviews.apache.org/r/18606/#comment66645> Yea, this is weird. Should we switch the ordering of these to make it more obvious that the Throwable is caught before falling into the `case e` block? samza-core/src/test/scala/org/apache/samza/util/TestUtil.scala <https://reviews.apache.org/r/18606/#comment66648> I *think* the logic for this function is supposed to be: 1. run block() 2. If block throws exception, the exception should be of the same class as "exception". 3. If the block throws exception, and it matches "exception" and msg is defined, then msg should match exception's message. 4. Fail if block doesn't throw exception, or if exception doesn't match "exception" param, or if msg is defined, and doesn't match exception.message. This method seems pretty jacked up. - Chris Riccomini On Feb. 28, 2014, 3:28 a.m., Jakob Homan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18606/ > ----------------------------------------------------------- > > (Updated Feb. 28, 2014, 3:28 a.m.) > > > Review request for samza. > > > Bugs: SAMZA-161 > https://issues.apache.org/jira/browse/SAMZA-161 > > > Repository: samza > > > Description > ------- > > SAMZA-161 > > > Diffs > ----- > > build.gradle 31c54be > gradle/dependency-versions-scala-2.10.gradle 47de65a > gradle/dependency-versions-scala-2.9.2.gradle e7b56a6 > gradle/license.gradle b4b62eb > > samza-core/src/main/scala/org/apache/samza/config/DefaultChooserConfig.scala > 9351c66 > > samza-core/src/main/scala/org/apache/samza/serializers/CheckpointSerde.scala > 0d82183 > samza-core/src/main/scala/org/apache/samza/system/SystemConsumers.scala > dfa3cd7 > samza-core/src/test/scala/org/apache/samza/util/TestUtil.scala 60c9615 > samza-kafka/src/main/scala/org/apache/samza/system/kafka/BrokerProxy.scala > a60de10 > > samza-kafka/src/test/scala/org/apache/samza/config/TestKafkaSerdeConfig.scala > 2e7459e > > samza-kafka/src/test/scala/org/apache/samza/system/kafka/TestKafkaSystemFactory.scala > 2a74ea5 > > samza-test/src/main/java/org/apache/samza/test/integration/SimpleStatefulTask.java > 973b0ba > > samza-test/src/main/java/org/apache/samza/test/integration/StatePerfTestTask.java > 873de74 > > samza-test/src/main/java/org/apache/samza/test/integration/join/Checker.java > 22f5e87 > > samza-test/src/main/java/org/apache/samza/test/integration/join/Emitter.java > 2989ca7 > > samza-test/src/main/java/org/apache/samza/test/integration/join/EpochPartitioner.java > d11d300 > samza-test/src/main/java/org/apache/samza/test/integration/join/Joiner.java > ca8fed4 > > samza-test/src/main/java/org/apache/samza/test/integration/join/Watcher.java > fac4ee1 > samza-test/src/main/resources/common.properties 971a219 > samza-test/src/main/resources/hello-stateful-world.samsa 84325d0 > samza-test/src/main/resources/join/checker.samsa e41ffa0 > samza-test/src/main/resources/join/emitter.samsa 140d13d > samza-test/src/main/resources/join/joiner.samsa 27655d8 > samza-test/src/main/resources/join/watcher.samsa a4cc761 > samza-test/src/main/resources/log4j.xml ecaf8a2 > samza-test/src/main/resources/perf/counter.samsa cf06c9e > > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterLifecycle.scala > b24f85a > > samza-yarn/src/test/scala/org/apache/samza/job/yarn/TestSamzaAppMasterTaskManager.scala > 3064c86 > > Diff: https://reviews.apache.org/r/18606/diff/ > > > Testing > ------- > > Unit tests > > > Thanks, > > Jakob Homan > >
