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

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

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

Reply via email to