Thanks Paul.
I had updated the webrev for your comments.

- Alex

On Jun 9, 2009, at 5:30 PM, Paul Cunningham wrote:

> Alex,
>
> This mainly looks good to me now, see below for a couple of minor  
> comments ...
>
> Paul
>
> alex zhang wrote:
>
>>> alex zhang wrote:
>>>> A latest webrev had uploaded at the same link:
>>>> http://cr.opensolaris.org/~alz/bittorrent/
>
>>>>>>>>> From: alex zhang <Huawei.Zhang at Sun.COM>
>>>>>>>>> Date: April 1, 2009 4:38:05 PM GMT+08:00
>>>>>>>>>
>>>>>>>>> Please help me have a code review of opensolaris package  
>>>>>>>>> porting, the webrev at:
>>>>>>>>> http://cr.opensolaris.org/~alz/libtorrent/, which had sent  
>>>>>>>>> out a few days without any response.
>
> 1. usr/src/lib/libtorrent/METADATA
>   Align the stuff on the PROJECT_URL: line with the other lines
>
Done, Now I change both METADATA files, they aligned when tabstop=8.

> 2. usr/src/cmd/rtorrent/Makefile.sfw
>   Lines ...
>     31 CONFIG_OPTIONS= CXX=$(CCC)
>     62       ./configure $(CONFIG_OPTIONS))
>   I still don't understand why you need the ...
>        ./configure CXX=$(CCC
>   when it is also set on line ..
>     58           "CXX=$(CCC)" \
>
> 3. usr/src/cmd/rtorrent/Makefile.sfw
>   Also for line ...
>     62       ./configure $(CONFIG_OPTIONS))
>   use $(SHELL), ie.
>       $(SHELL) ./configure ....
>   and explicitly set the --prefix= value using
>       $(SHELL) ./configure $(CONFIGURE_OPTIONS) ...
>   where CONFIGURE_OPTIONS is defined in Makefile.master
Now I remove the define of CONFIG_OPTIONS of myself.
It works ok.

>
> END
> -- 
> ----------------------------------------------------------------------
> Paul Cunningham
> Software Engineer
> Tadpole Business Unit


Reply via email to