See below ... Paul
Abhijit Nath wrote: > > Thanks for your review comments. I have incorporated all except comment 3. > On 03/19/09 17:08, Paul Cunningham wrote: >> Abhijit Nath wrote: >>> >>> Please help review the changes to integrate DOSBox v. 0.72 (CR >>> 6657028). I have incorporated the basic review comments. >>> The webrev is at: http://cr.opensolaris.org/~an230044/dosbox/ >> >> === Start of Comments === .. cut .. >> 2. CDDL HEADER and top of files (various files) >> Cosmetic: Change the tops of files so that they >> conform to that in .. >> "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; Various files are still wrong, eg. a) extra # line at top b) missing line-space after CDDL HEADER END header >> 3. usr/src/cmd/dosbox/Makefile.sfw >> Change as follows .. >> Roland Mainz wrote: >> > use "env - ..." and not "env ..." in the Makefiles >> > to make sure "configure"&&"make" only see the env >> > variables they should really get (and not pick-up >> > any random env variable). >> > use either $(SHELL) or /usr/bin/bash for "configure" >> > calls (so we know which one is used and "configure" >> > doesn't pick one itself). As per previous reply; you should apply this Do you still need the line ... 56 INSTALL="$(INSTALL) -c" .. cut .. >> 5. usr/src/cmd/dosbox/sunman-stability >> Is the following state correct ? .. >> 22 Interface Stability Volatile Is 'Volatile' really the correct state for this ? .... cut ... >> 9. usr/src/pkgdefs/SUNWdosbox/prototype_com >> I assume this only works on i386, so shouldn't the >> files be in SUNWdosbox/prototype_i386 rather than in >> here ? What was your decision here ? Also if it is only i386, in usr/src/pkgdefs/Makefile shouldn't 'SUNWdosbox' be in the i386_SUBDIRS= list rather than the COMMON_SUBDIRS= list ? The same probably also applies to usr/src/cmd/Makefile >> === End of Comments ===== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
