Mayuresh,

See below for my comments from my quick-ish skip through ...

Paul

Mayuresh Nirhali wrote:
> 
> I am integrating wxWidgets shared libraries into SFW.
> Please note that, wxWidgets already exists in SFW, but in static form 
> (for pgAdmin3).
> This code review is to expose the wxGTK libraries as a shared component, 
> so that other GUI tools can also use them (e.g. audacity, filezilla).
> 
> I would really appreciate a code review from the community for my changes.
> 
> The webrev lives at,
> http://cr.opensolaris.org/~wittyman/wxwidgets/
> 
> and the bug report is at,
> http://bugs.opensolaris.org/view_bug.do?bug_id=6739008
> 
> References:
> wxWidgets: A cross-platform GUI toolkit
> http://www.wxwidgets.org/

==== Start of Comments ===

1. usr/src/cmd/postgres/pgadmin/Makefile.sfw
     & usr/src/lib/wxwidgets/Makefile.sfw
    As you are changing these you might want to use the
    predefined 'configure --prefix=...' value from
    Makefile.master and you could also set up the
    configure options using CONFIGURE_OPTIONS method,
    eg., something like ...
     CONFIGURE_OPTIONS += --bindir=$(CFGPREFIX)/pgadmin3/bin
     CONFIGURE_OPTIONS +=  --libexecdir=/usr/pgadmin3/bin
     ...
     ...
     ./configure $(CONFIGURE_OPTIONS)
    There should be various examples of doing it this
    way in the gate now, eg. cups, etc.

    You also might want to use 'env - ' rather than 'env '

2. usr/src/cmd/postgres/pgadmin/Makefile.sfw
    Shouldn't the 'CPPFLAGS=...' line be before the
    './configure' line ?

3. usr/src/lib/wxwidgets/man/wx-config.1
     & usr/src/lib/wxwidgets/man/wxrc.1
    Do these need the sun stability stuff added in someway;
    either in this files or via sunman-stability script?

4. usr/src/lib/wxwidgets/install-sfw
    Again as you are changing this file you might want to
    look at 'Roland Mainz' comments on other people's
    install-sfw scripts and apply those comments - look
    back through sfwnv-discuss for them.

    And do all these files really need the write permissions
    bit set? (and in prototype* files)

5. usr/src/pkgdefs/SUNWwxwidgets-devel/Makefile
      & usr/src/pkgdefs/SUNWwxwidgets/Makefile
    You could remove the null DATAFILES= line

6. usr/src/pkgdefs/SUNWwxwidgets-devel/pkginfo.tmpl
     & usr/src/pkgdefs/SUNWwxwidgets/pkginfo.tmpl
    Its more normal to put the version number at the end
    of the DESC= line (not that it really matters)

7. everything else looks okay to me

==== End of Comments =====

-- 
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products

Reply via email to