[7u4-osx] Please review: 7124089: launcher refactoring v2.0

Kelly O'Hair kelly.ohair at oracle.com
Mon Jan 23 09:07:35 PST 2012


With Makefiles, I have tended to use 2 spaces for indentation because using 4 quickly puts you at 8, which might trigger
a TAB character with some editor, which will cause strange errors in some cases.
You can still get to 8 using 2, but less likely.

Not sure this 2 space indentation rule for makefiles is written down anywhere. :^(

-kto

On Jan 22, 2012, at 6:14 PM, David Holmes wrote:

> On 22/01/2012 4:05 AM, Kumar Srinivasan wrote:
>> Hi Kelly et. al.,
>> 
>> I have beautified/fixed the Makefiles addressing Kellys' comments below:
>> 
>> 1. Indented the Makefiles correctly.
> 
> By some definition of "correct". The existing indentation is all over the place with 2, 4 and 8 space indents. The changes seem to use 4 most often but the end result is still a mix of 2,4 and 8 AFAICS. :(
> 
> David
> -----
> 
>> 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
>>>> 
>>>> 
>> 




More information about the jdk7u-dev mailing list