Stefan, See below for a few comments ....
Paul Stefan Teleman wrote: > http://cr.opensolaris.org/~steleman/6937593/ > > Thank you! 1. The new and old tarball do not show up in the webrev, to indicated what is happening to them. 2. usr/src/lib/lcms/Makefile.sfw Could the patches (4 of them) ... gpatch -p0 <../libtool.1.patch be done with the other patches at the bottom with the tarball uncompresses? Lines .. 83 all: real-all 274 real-all: all32 all64 lint32 lint64 could that just be .. all: all32 all64 lint32 lint64 Line ... 35 VER=lcms-$(RELNUM) could you extract the lcms from the METADATA NAME field ? Line ... 276 test: test32 test64 do you need this, as there are no test32 test64 rules ? Line ... 41 PREFIX = /usr use the CFGPREFIX defined in Makefile.master rather than hard coding /usr Lines ... 52 PYTHON24 = /usr/bin/python2.4 53 PYTHON26 = /usr/bin/python2.6 could you use the ones defined in Makefile.master Lines ... 54 PYTHON_PATH_24 = /usr/lib/pytyhon2.4/vendor-packages 55 PYTHON_PATH_24_64 = /usr/lib/pytyhon2.4/vendor-packages/64 56 PYTHON_PATH_26 = /usr/lib/pytyhon2.6/vendor-packages 57 PYTHON_PATH_26_64 = /usr/lib/pytyhon2.6/vendor-packages/64 is that a a typo ? pytyhon ? Lines (4 if them) ... "./configure ..... \ ..... \ etc ..." could you use the CONFIGURE_OPTIONS method of setting the options rather than duplicating them 4 times, eg .. CONFIGURE_OPTIONS += --localstatedir=/var CONFIGURE_OPTIONS += --enable-shared etc ..... ..... ./configure $(CONFIGURE_OPTIONS) ..... ..... ./configure $(CONFIGURE_OPTIONS) $(CONFIGURE_OPTIONS64) Lines ... 390 cd ..) 400 410 420 does it really need these lines? Lint stuff .. is this required ? 3. usr/src/pkgdefs/SUNWlcms/depend The copyright lines should probably be moved to after the "CDDL HEADER END" header 4. usr/src/lib/lcms/METADATA Why the "==========..." line (line 14)? Everything else looks okay to me -- Paul Cunningham Software Engineer
