I fixed all the indenting in the build scripts to be 2 for consistency,
removed tabs in conditionals (vi's set list revealed them), and
some minor cleanups.

Here is the incremental webrev:
http://cr.openjdk.java.net/~ksrini/7124089/webrev.3/webrev.delta/index.html

Full webrev:
http://cr.openjdk.java.net/~ksrini/7124089/webrev.3/index.html

I hope I am good to go now ?
Thanks
Kumar

Hi Kelly et. al.,

 I have beautified/fixed the Makefiles addressing Kellys' comments below:

1. Indented the Makefiles correctly.
2. Annotated with more  trailing comments to the if/else/endif clauses
3. Removed the offending \ escapes
4. Removed the * from Release.gmk, it turns out the files being copied
   were not quite right (missing files), fixed it such that all the
appropriate files
   are copied.
5. Added comments for the MacOSX specific cflags.

The incremental webrev is here:
http://cr.openjdk.java.net/~ksrini/7124089/webrev.2/webrev.delta/index.html


The full webrev is here:
 http://cr.openjdk.java.net/~ksrini/7124089/webrev.2/index.html

Thanks
Kumar

On the Makefiles....

Please refrain from using any wildcards (e.g. * ) in the make rules.
Better to be explicit, or if necessary
use something like FILES=$(wildcard $(SOMEDIR)/*) and a cp $(FILES)
$(SOMEPLACE)
so that we can at least see in the Makefile log what it really copied.

Please indent the Makefile if/else/endif statements.

Thank you for the trailing comments on the endif's. ;^)

Please try to avoid escaped quotes on the compile lines, use this
-DX='"abc"' rather than this -DX=\"abc\"
escaped quotes are very problematic on Windows and I know this isn't
Windows, but it tempts windows
people to use it, it will not work in all situations. Where '"abc"'
does.

Please add a comment on what the -Os compiler option means, and also
the -x objective-c, I could guess
but would be better to document it in the makefile.

-kto

On Jan 20, 2012, at 8:24 AM, Kumar Srinivasan wrote:

Hi All,

Based on all the comments from Anthony, Joe and David,
here is the modified version:

Highlights:
1. re-factored code in solaris directory  to be shared with macosx,
    reducing duplication across the *nixes.

2. adjusted the makefilesto allow the above

2. eliminated all conditionals from the shared  java.c

3. added a new launcher regression test for the macosx specific -X
options

For those who have already reviewed the 0th version, here is an
incremental webrev to make it easier reviewing the differences.
http://cr.openjdk.java.net/~ksrini/7124089/webrev.1/webrev.delta/index.html


Here is the complete webrev:
http://cr.openjdk.java.net/~ksrini/7124089/webrev.1/index.html

Thanks
Kumar




Reply via email to