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

Reply via email to