Looks good to me

paul :-)

Robin Guo wrote:
> 
>   That's true, and sorry for I modified it too fast before left office
> yesterday and not try rebuild on my machine. I've updated it by your 
> comments
> and verified the build process works okay.
> 
>   Webrev is refreshed, thanks for all you kindly help and comments.
> 
>   - Regards,
> 
> Paul Cunningham wrote:
>> Robin,
>>
>> Robin Guo wrote:
>>> I've updated the webrev according to all your comment tips,
>>> please refresh the page and see how it going?
>>>
>>>   Thanks.
>>
>> That's okay
>>
>> But see additional comments below ....
>> Everything else looks okay.
>>
>> Paul
>>
>>> Paul Cunningham wrote:
>>>> Robin Guo wrote:
>>>>> I just put a code review for tcpdump package porting,
>>>>> would you or somebody else have time to take a look on it?
>>>>>
>>>>> The webrev is at http://cr.opensolaris.org/~robinguo/tcpdump/
>>
>>>> === Start of Comments ====
>>
>> 1A. usr/src/cmd/Makefile & usr/src/pkgdefs/Makefile
>>     These needs resyncing with the gate so it doesn't
>>     look as though you are adding/deleting stuff that's
>>      nothing to do with your pkg.
>>
>>   .... cut ...
>>
>>>> 3. usr/src/cmd/tcpdump/Makefile.sfw
>>>>    Extract the VER= info from the METADATA, see recent
>>>>    integrations for examples.
>> Shouldn't line ...
>>   28 VER =           $(COMPONENT_VERSION:sh)
>>   29 TARBALL =       $(VER).tar.gz
>> be ? ...
>>      VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh)
>>      TARBALL =$(VER).tar.gz
>>
>>      ... cut ...
>>
>>>> 4. usr/src/cmd/tcpdump/install-sfw
>>   .. cut ..
>>>>
>>>>    Pass the PKGVERS= info in from your Makefile.sfw (extracted
>>>>    from METADATA) as an environment var or as an option.
>>
>> Does this need a ...
>>    cd <to-somewhere>
>> before the first '_install ....' line, eg. something like ..
>>   37 source $SRC/tools/install.subr
>>   38
>>      cd ${VER}
>>
>>   39 _install M tcpdump.1 ${MANDIR}/man1/tcpdump.1 444
>>   40
>>
>>     ... cut ...
>>
>>>> === End of Comments ======
-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit

Reply via email to