[7u4] Review request for CR 7134730 (original) (raw)

Kumar Srinivasan kumar.x.srinivasan at oracle.COM
Mon Feb 13 10:46:28 PST 2012

4. I also find the version string hard-coded in build.xml, we need to have a mechanism such that version information is derived from the make/build logic. Do you mean the version that we get from build.properties? It looks like you have a todo for this as well, you can always tunnel in the version information from make into ant, the langtools make and ant have examples of these, if the ant is going to be run with built jdk, then you can infer these from the ant system itself. http://hg.openjdk.java.net/jdk7u/jdk7u-dev-gate/langtools/file/4672e092f096/make/Makefile

50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66

Kumar

5. The test is great, but how will the test be executed ? typically in the jdk, the tests are located under jdk/test/......, and these are all run by jtreg automatically on a periodic basis (nightly, weekly, promotion etc), we need this hooked up into that framework. You are correct - enabling automated unit tests for this code is still a TODO. 6. AppBundlerTask.java.html

Does the ant code get built and run with jdk6 ? No, Java 7 is required. it that is the case then the following may be moot, but of course you can use for-each and simplify some code below. 71 private ArrayList classPath = new ArrayList(); 72 private ArrayList nativeLibraries = new ArrayList(); 73 private ArrayList options = new ArrayList(); 74 private ArrayList arguments = new ArrayList(); you can set the assiged type to List. private List classPath = new ArrayList<>(); That's true. I haven't fully acclimated myself to Java 7 syntax yet. :-) lines 135.... 135 public void addConfiguredClassPath(FileSet classPath) { 136 File parent = classPath.getDir(); 137 138 DirectoryScanner directoryScanner = classPath.getDirectoryScanner(getProject()); -139 String[] includedFiles = directoryScanner.getIncludedFiles(); 140 -141 for (int i = 0; i< includedFiles.length; i++) { -142 this.classPath.add(new File(parent, includedFiles[i])); -143 } for (String name : directoryScanner.getIncludedFiles() { this.classPath.add(new File(parent, name); } 144 } Thanks for this suggestion. Apparently I'm not yet fully acclimated to Java 7 APIs, either. ;-) similarly lines 453 and 477. Test.java, I would make the test program dump out the args as well, and pass some dummy args to ensure that args are passed correctly to the app's main. Good idea. I noticed that you set the starting heapsize and maximum heap size ? That's just to test setting VM options using the launcher. I can change them to something else if there are better test options to use. G

More information about the macosx-port-dev mailing list