I've replied to some of your things, see below ... I will take another look at the updated webrev tomorrow morning (UK time), assume I get time.
Jim, please answer a few things below (thanks) Bharath Kumar wrote: > I have updated the webrev with changes as per your comments. Response > inlined > Webrev: http://cr.opensolaris.org/~bkumarm/imperius/ > > Paul Cunningham wrote: ... cut .. >> Why is this ... >> 36 JAVA_ROOT=/usr/jdk/j2sdk1.4.2_06 >> different to the JAVA_ROOT in Makefile.master ? > > Since Imperius builds only with JDK.1.4.2, I have had to explicitly set > the path It may be worth adding a comment to say that > >> 4. CDDL HEADER and top of files >> You may want to change all the files so that they conform >> to that in ... >> "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/" >> (see http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines) > > In another review (httping), Jim had specifically asked for the removal > the extra blank line. Hence followed the same. Not that it really matters, but I think he was asking for removal of double space chars and double line spaces. Its just nice to get them all consistent especially on new integrations - Check with Jim though. >> 6. usr/src/lib/imperius/antlr-2.7.7.jar >> Where is this used ? >> > antlr-2.7.7.jar is used to build the imperius-splcore grammar files. > This is required only at build time. How does that actually get included in the build, its no obvious to me - may be add comment to Makefile.sfw to say how. But I maybe being thick :-[ here. >> 7. usr/src/pkgdefs/SUNWImperius/copyright >> Add the pkg originator copyrights at the top? See >> http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright >> >> > Checked a few packages having Apache license (SUNWant, SUNWapch22u, > SUNWapch22r) and followed the same > http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWant/copyright > http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWapch22u/copyright > > http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWapch22r/copyright > That does mean that they are right though - again check with Jim Walker Paul -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
