Mayuresh, More comments below from quick skip through ...
Paul Mayuresh Nirhali wrote: > > 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. > > Paul Cunningham wrote: >> 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 ' You might want to apply 'Roland Mainz' comments ... - Use "env - ..." and not "env ..." in the Makefiles to make sure "configure" & "make" only see the env variables they should really get(and not pick-up any random env variable) - Use either $(SHELL) or /usr/bin/bash for "configure" calls (so we know which one is used and "configure" doesn't pick one itself). (done in wxwidgets/Makefile.sfw) >> >> 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? In usr/src/lib/wxwidgets/install-sfw I think you need to change the '_install N ..' to '_install M ..' so you get the sunman-stability stuff added. >> >> 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) Still got write bit set on /usr files? 644 and in .. usr/src/pkgdefs/SUNWwxwidgets-devel/prototype_com >> >> 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 General Dynamics Itronix Europe Ltd. Pioneer House Chivers Way Histon, Cambridgeshire, UK, CB24 9NL Ph: +44 (0)1223 200648 FAX: +44 870 4324162 Email: paul.cunningham at tadpole.com This email message is for the sole use of the intended recipient(s) and may contain GDC4S confidential or privileged information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message
