----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19370/#review37957 -----------------------------------------------------------
twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/BundledJarExample.java <https://reviews.apache.org/r/19370/#comment69802> So you join it to one String argument? Shouldn't it be String[]? twill-ext/pom.xml <https://reviews.apache.org/r/19370/#comment69805> You shouldn't need version as it should be inherited. twill-ext/pom.xml <https://reviews.apache.org/r/19370/#comment69806> Same, no version needed. twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunnable.java <https://reviews.apache.org/r/19370/#comment69807> protected rather than public? Also, maybe better call it doInitialize twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunnable.java <https://reviews.apache.org/r/19370/#comment69808> Since you have the subInitialize method for children class to override, I assume to want to fix the implementation of this method, which mean this method should be marked "final". twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunnable.java <https://reviews.apache.org/r/19370/#comment69809> Wrong alignment. twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunnable.java <https://reviews.apache.org/r/19370/#comment69812> What's the purpose of this setter? Who will call this? twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunnable.java <https://reviews.apache.org/r/19370/#comment69813> Seems like all these getter are for sub-class only? If that's the case, make them protected. twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java <https://reviews.apache.org/r/19370/#comment69815> It seems the throws signature is pretty long. How about just throws Exception? - Terence Yim On March 18, 2014, 8:52 p.m., Jiahua Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19370/ > ----------------------------------------------------------- > > (Updated March 18, 2014, 8:52 p.m.) > > > Review request for Twill. > > > Repository: twill > > > Description > ------- > > BundledJarRunnable is a TwillRunnable that runs bundled jars. > > > Diffs > ----- > > pom.xml 8e93850 > twill-api/src/main/java/org/apache/twill/api/LocalFile.java bcc3e13 > twill-examples/echo/pom.xml PRE-CREATION > twill-examples/echo/src/main/java/echo/EchoMain.java PRE-CREATION > twill-examples/pom.xml PRE-CREATION > twill-examples/yarn/pom.xml PRE-CREATION > > twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/BundledJarExample.java > PRE-CREATION > > twill-examples/yarn/src/main/java/org/apache/twill/example/yarn/HelloWorld.java > PRE-CREATION > twill-ext/pom.xml PRE-CREATION > twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunnable.java > PRE-CREATION > twill-ext/src/main/java/org/apache/twill/ext/BundledJarRunner.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/19370/diff/ > > > Testing > ------- > > BundledJarExample and Presto application. > > > Thanks, > > Jiahua Wang > >
