Hi Vivek, I started to look at this but there is too much of it to do in one or even two goes, so please could you split the webrev up into its eight (I think) component parts and send them out separately (it doesn't look as though they need to be done in one go to me).
Below are a couple of points you might want to consider ... Also I assume this collection of components that make up 'Drools' is this application "http://en.wikipedia.org/wiki/Drools"; ? Paul Vivek Titarmare wrote: > > I have posted a webrev for packages "janino", "jettison", "jaxen-core", > "jodatime", "mvel", "relaxngDatatype", "staxapi" & "xpp3min" which I am > porting to OpenSolaris and would like to request a code review. > > Please see http://cr.opensolaris.org/~vivekrt/Drools-B1/ and provide any > comments as needed if there are any issues which I need to correct. 1. METADATA You have ... BUGTRAQ: solaris/utility/drools wouldn't it be better if each component part had its own category, eg .. BUGTRAQ: solaris/utility/janino or if you can have this maybe .. BUGTRAQ: solaris/utility/drools/janino 2. install-sfw In the install-sfw scripts I looked at you have the pkg name and its version hardcoded in the scripts, it might be better if you pass this in from the Makefile.sfw as an env variable (or option), eg. in janino change ... janino/Makefile.sfw 43 VERS=$(VER) $(SHELL) ./install-sfw janino/install-sfw 66 cd ${VERS}/dist 69 _install N ${VERS}.jar ${JARDIR}/${VERS}.jar 444 etc. You will then not have to keep changing them every time the pkg version is updated. 3. Drools top level How are all the component pkgs pulled together to ensure anyone who wants to install Drools has all the component parts installed? -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
