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 >>>> >>>> >>
