> On July 30, 2017, 5:05 a.m., Jinmei Liao wrote: > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilder.java > > Lines 66 (patched) > > <https://reviews.apache.org/r/61166/diff/4/?file=1785455#file1785455line66> > > > > looks like only the JavaCompiler needs a temp dir to store some > > temporary files and delete them afterwards. JarBuilder doesn't need it. Can > > we have JavaCompiler uses a system temp folder instead of needing a dir to > > construct it?
My concern was that the built-in `File.createTempFile` method doesn't clean up after itself as reliably as the JUnit `TemporaryFolder` rule. I'll add a no-arg constructor that uses a system temp folder and also sets `deleteOnExit`. > On July 30, 2017, 5:05 a.m., Jinmei Liao wrote: > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilderTest.java > > Lines 105 (patched) > > <https://reviews.apache.org/r/61166/diff/4/?file=1785456#file1785456line105> > > > > this method seems useful, can we allow user to simply pass a className > > (with package prefixes) instead of a File object to the JarBuilder? > > > > Basically I am wondering if the usage of the JarBuilder can be simplied > > to the following: > > > > JarBuilder jarbuilder = new JarBuilder(); > > File outputFile = new File(tempDir, output.jar); > > jarBuilder.buildJar(outputFile, "com.foo.Bar1", "com.foo.Bar2") Great suggestion! I will add this functionality. > On July 30, 2017, 5:05 a.m., Jinmei Liao wrote: > > geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ExtendsFunctionAdapter.java > > Lines 19 (patched) > > <https://reviews.apache.org/r/61166/diff/4/?file=1785464#file1785464line19> > > > > this has no package declaration. Is it intended? I omitted a package declaration for simplicity since it didn't seem necessary to specify one. I can add one in if you think the lack of a package is confusing. - Jared ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61166/#review181770 ----------------------------------------------------------- On July 28, 2017, 10:46 p.m., Jared Stewart wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61166/ > ----------------------------------------------------------- > > (Updated July 28, 2017, 10:46 p.m.) > > > Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and > Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > GEODE-3313: Test utility supports building jar files with multiple classes > > > Diffs > ----- > > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/ClassNameExtractor.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/ClassNameExtractorTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/CompiledSourceCode.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilder.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JarBuilderTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JavaCompiler.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/JavaCompilerTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/deployment/compiler/UncompiledSourceCode.java > PRE-CREATION > > geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/AbstractExtendsFunctionAdapter.java > PRE-CREATION > > geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/AbstractFunction.java > PRE-CREATION > > geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ConcreteExtendsAbstractExtendsFunctionAdapter.java > PRE-CREATION > > geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ExtendsAbstractFunction.java > PRE-CREATION > > geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ExtendsFunctionAdapter.java > PRE-CREATION > > geode-core/src/test/resources/org/apache/geode/management/internal/deployment/compiler/ImplementsFunction.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/61166/diff/4/ > > > Testing > ------- > > Precheckin running > > > Thanks, > > Jared Stewart > >
