Hi Steve,
Please see inline for the comments. If its fine, I'l send the new webrev
with Targetdirs.
Steven M. Christensen wrote:
> Spoorthy,
>
> I see several issues.
>
> You have made changes to usr/src/Targetdirs, but I don't see this in
> your webrev.
Added.
>
> Also, in your install-sfw you have an /etc/init.d directory, but this
> is not in Targetdirs. What about the lib/examples directory or
> lib/conman?
They are included in Targetdirs as
/usr/lib/conman \
/usr/lib/conman/examples \
/usr/lib/conman/exec \
>
> You have
>
> _install E conman ${ROOT}/etc/logrotate.d/conman 555
> _install E conman ${ROOT}/etc/default/conman 555
> _install N etc/conman.sysconfig ${ROOT}/etc/default/conman.sysconfig 744
> _install E conman ${ROOT}/etc/init.d/conman 555
>
> in install-sfw.
>
> Why are the
>
> logrotate.d/conman
> default/conman
>
> files E and 555? These are not executables. Should they not be
> N 444 or maybe 644? This would carry over into the prototype_com
> files as well.
>
They are the executables.
> Is there a depend file for SUNWconmanr? There is one for SUNWconmanu.
>
No depend file for SUNWconmanr. It does not depend on any packages.
SUNWconmanu depends on root package - SUNWconmanr.
Thanks
Spoorthy
> That's all I see right now.
>
>
> Steve C.
Spoorthy H.S wrote:
>> Hi Paul,
>>
>> New webrev is at http://cr.opensolaris.org/~spoorthy/conmanupd1/
>> and comments inline..
>>
>>
>> Paul Cunningham wrote:
>>> Spoorthy,
>>>
>>> See below for my comments ...
>>>
>>> Paul
>>>
>>> Spoorthy H.S wrote:
>>>>
>>>> New webrev is at
>>>> http://cr.opensolaris.org/~spoorthy/conmanupd/
>>>
>>> === Start of Comments ===
>>>
>>> 1. usr/src/Targetdirs
>>> Put your new bits in alphabetically ...
>>> /etc/apache2/2.2 \
>>> /etc/apache2/2.2/conf.d \
>>> + /etc/logrotate.d \
>>> + /etc/default \
>>> /etc/lighttpd \
>>> /etc/lighttpd/1.4 \
>>> /etc/lighttpd/1.4/conf.d \
>>> /etc/openwsman \
>>>
>> Done
>>> 2. usr/src/cmd/conman/METADATA
>>> Remove the last blank line
>>>
>>> Complete this line ...
>>> 11 BUGTRAQ:
>>>
>> Raised the request. Will update the field.
>>> 3. usr/src/cmd/conman/Makefile.sfw
>>> On line 33 you change ...
>>> --sysconfdir=/etc
>>> to ..
>>> --sysconfdir=$(CFGETC)
>>>
>>> Cosmetic: move the line-space up a line, eg. ..
>>> 19 # CDDL HEADER END
>>> 20 #
>>> ..
>>> 21 #
>>> 23 # Copyright 2009 Sun Microsystems, Inc. All rights reserved.
>>>
>> Done
>>> 4. usr/src/cmd/conman/install-sfw
>>> Cosmetic: add extra line-space after line 45
>>>
>>> On lines 74 to 83 you could use ${LIB} instead of
>>> ${PREFIX}/lib
>>>
>> Declared variable is used.
>>> 5. Various SUNWconmanr/* and SUNWconmanu/* files
>>> Change the top of these files so they all conform to
>>> that in ...
>>> "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/";
>>>
>>>
>>> 6. usr/src/pkgdefs/SUNWconmanr/pkginfo.tmpl
>>> As this is a root package you may want to remove the
>>> pkg version number on the DESC= line (that way this
>>> file should not need changing when the src pkg
>>> is up versioned)
>>>
>> Removed the version number.
>>> 7. usr/src/pkgdefs/SUNWconmanr/prototype_com
>>> Do any of the installed /etc files need preserving
>>> over a SUNW pkg upgrade? If so you need to handle that.
>>>
>> I dont think it happens.
>>> 8. pkgdefs/Makefile
>>> & cmd/Makefile
>>> The changes to these are not in the webrev
>>>
>> Done
>>> 9. usr/src/pkgdefs/SUNWconmanu/copyright
>>> Add src pkg owner copyright lines (after sun disclaimer)
>>> extracted from the src files in the uncompressed tarball,
>>> see example at ...
>>> "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright";
>>>
>>>
>>>
>> The copyright file is "COPYING" in my package. I copied the contents
>> of it to "copyright" file after SUN disclaimer.
>>> 10. usr/src/pkgdefs/SUNWconmanu/depend
>>> Are there any other dependencies?
>>>
>> No other dependencies except Root pakage - SUNWconmanr. Do let me
>> know how can I run the "package dependency checker".
>>
>> Thanks
>> Spoorthy
>>> === End of Comments =====
>> _______________________________________________
>> sfwnv-discuss mailing list
>> sfwnv-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
>>
