Hi David,
The updated webrev looks fine. Just couple of things...
1. In install-sfw file please check if the permissions on the config
files as well as license need to be 555.
2. In install-sfw, while creating a link, you needn't specify the
permission
Please change the below
_install L ${PREFIX}/areca.sh ${ROOT}/usr/bin/areca 755
to
_install L ${PREFIX}/areca.sh ${ROOT}/usr/bin/areca
3. Since you are now making use of the default depend file (please
ensure that you have checked for the project specific dependencies),
there is no need to include the depend file in the webrev.
4. Please make sure that you resync the workspace.
An entry for the dirs that you are creating in install script using
mkdir -p can be included in the usr/src/Targetdirs file. For eg, if you
need to create a dir called 'usr/share/areca' then this entry can be put
in the usr/src/Targetdirs file in alphabetical order. Please see
previous integrations as examples.
Regards
Muktha
David Zhang wrote:
> Hi Muktha,
>
> Thank you very much for your detailed comments, I have addressed most
> of them and post it on http://cr.opensolaris.org/~ydzhang/areca/.
>
> However, I have questions for 4.b using (Targetdirs) instead of 'mkdir
> -p'. Would you please send me an example?
>
> Regards,
> David
>
> On 04/24/09 22:26, Muktha Narayan wrote:
>> Hi David,
>>
>> Few review comments:
>>
>> 1. cmd/Makefile
>> The entries should be in alphabetical order.
>> 2. As per the website, areca 7.1 has been released, is there a
>> specific reason why 7.0.8 is being integrated?
>> 3. Makefile.sfw
>> a) 'pragma ident' keyword is generally not used in files other
>> than source files. In Makefiles, install scripts and such, use
>> "ident" (without "pragma").
>> b) You can avoid hard-coding program name and version and instead
>> extract this info from METADATA, something like:
>> VER = $(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh)
>> 4. install-areca
>> a) The install files are usually named as install-sfw. b) The
>> dirs created by 'mkdir -p' should be in the Targetdirs file so you
>> don't have to create it here.
>> c) Please check if the files installed (icons, translations) need
>> to have 555 permission.
>> 5. depend
>> a) Please check if there are any missing dependencies by running
>> the check-deps.pl script or by doing "make check_deps" in
>> usr/src/pkgdefs/SUNWareca dir.
>> b) If there are no project specific dependency then the default
>> depend file can be used by including the below line in
>> SUNWareca/Makefile
>> DATAFILES = depend
>> 6. pkginfo.tmp
>> a) You could add the pkg version number at the end of the DESC.
>> DESC="Areca- backup utilities (7.0.8)"
>> 7. prototype_com
>> a) Please check the permission on the directories it should be 0755.
>> b) Please check if pngs and *.properties files need 0555 permission.
>>
>> Regards
>> Muktha
>>
>> David Zhang wrote:
>>> Hi experts,
>>>
>>> Would you please kindly review those webrev for opensolaris package
>>> SUNWareca?
>>> http://cr.opensolaris.org/~ydzhang/areca/
>>>
>>> Regards,
>>> David
>>>
>>>
>>> _______________________________________________
>>> sfwnv-discuss mailing list
>>> sfwnv-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
>>>
>>
>
