Paul, Thanks for your valuable comments. and I apologize for not having sent the updated webrev sooner.
I have addressed your comments and the modified webrev is living at, http://cr.opensolaris.org/~wittyman/wxwidgets/ Apart from your comments, there are some minor modifications in the 'depend' file. I request everyone on this list to review this code change. Thanks in advance Mayuresh Paul Cunningham wrote: > 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 ===== >
