Simon,

More comments ...

Paul

lingjun wang (Simon) wrote:
> 
> See my inline comments.
> 
> Thanks very much for your review.
> 
> Paul Cunningham :
>>
>> Mainly looks good to me, just few minor comments ...
>>
>>
>> Ling-Jun Simon Wang wrote:
>>
>>> <a href=http://cr.opensolaris.org/~lw202042/smartmontools/>code 
>>> review for smartmontools</a>
>>>  
>>
>>
>> ====== Start of Comments ================
>>
>> 1. usr/src/pkgdefs/Makefile
>>    You seem to have removed some apache stuff, did u mean to?
>>
> Actually I didn't remove any stuff here. Instead, I need a bringover and 
> I did it.
> 
>> 2. usr/src/cmd/smartmontools/smartmontools.xml.in +
>>    usr/src/pkgdefs/SUNWgnu-smartmontools/depend
>>    You might want to move the copyright lines to after the
>>    "CDDL HEADER END"
>>
> Yes. I did it.
> 
>> 3. usr/src/pkgdefs/SUNWgnu-smartmontools
>>    You don't seem to have included the Makefile in the webrev
>>
> Yes. I include it now.

SUNWgnu-smartmontools/Makefile
I don't think you need the 'DATAFILES = depend' line if you have your 
own 'depend' file.

>> 4. usr/src/pkgdefs/SUNWgnu-smartmontools/pkginfo.tmpl
>>    You might want to remove the package version from the
>>    'DESC=' line.
>>
> Yes, I removed it.
> 
>> 5. usr/src/pkgdefs/SUNWgnu-smartmontools/prototype_com
>>    Should the 'etc' stuff be in a root package?
>>
> It should be in the root package.

What I was trying to get at is that the following should be in a 
separate SUNWgnu-smartmontoolsr (root) package I think (maybe) ..
   d none etc 0755 root sys
   f none etc/smartd.conf 0644 root sys
and maybe ..
   d none lib 0755 root bin
   d none lib/svc 0755 root bin
   d none lib/svc/method 0755 root bin
   f none lib/svc/method/svc-smartd 0555 root bin
   d none var 0755 root sys
   d none var/svc 0755 root sys
   d none var/svc/manifest 0755 root sys
   d none var/svc/manifest/application 0755 root sys
   f none var/svc/manifest/application/smartmontools.xml 0444 root sys

but check elsewhere

>> 6. usr/src/pkgdefs/SUNWgnu-smartmontools/prototype_sparc +
>>    usr/src/pkgdefs/SUNWgnu-smartmontools/prototype_i386
>>    name wrong in file - # SFWgnu ????
>>
> Yes, a typo. I corrected it.
> 
>> ====== End of Comments ==================
>>

-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products
General Dynamics Itronix Europe Ltd.
Pioneer House
Chivers Way
Histon, Cambridgeshire,
UK, CB24 9NL
Ph:  +44 (0)1223 200648
FAX: +44 870 4324162
Email: paul.cunningham at tadpole.com

This  email  message  is  for  the  sole  use of the intended
recipient(s) and may contain GDC4S confidential or privileged
information.  Any  unauthorized  review, use,  disclosure  or
distribution  is  prohibited.  If  you  are  not an  intended
recipient,  please  contact  the  sender  by reply  email and
destroy all copies of the original message

Reply via email to