Paul, Was busy with some other stuff, so the delay.
Please find the new set of webrev files for this project at : http://cr.opensolaris.org/~manjuhb/routes/ Please find my comments embedded in the email. -manjunath Paul Cunningham wrote: > See below ... > > paul > > Manjunath Basappa wrote: > >> >> Webrev files are located at http://cr.opensolaris.org/~manjuhb/mb117111/ > >>> >>> Please see the new code review webrev files. I have incorporated >>> the corrections you have mentioned. >>> >>> Note, we are now delivering two packages, for py 2.4 and py 2.6 >>> from the same source file. >>> >>> Please review and let me know your feedback. > >>>> >>>> Manjunath Basappa wrote: >>>>> >>>>> I am porting routes package to opensolaris /release repository. >>>>> >>>>> Please find attached the relevant documents. >>>>> >>>>> I also request a review of the source code. The webrev output is >>>>> located at http://cr.opensolaris.org/~manjuhb/webrev/. > >>>> 1. usr/src/lib/Routes2.4/METADATA > >>>> Why does the SOURCE_DOWNLOAD: url point to opensolaris.org, >>>> shouldn't it point to the official download site for this pkg? > > This should probably be ... > "http://pypi.python.org/packages/source/R/Routes/Routes-1.10.3.tar.gz"; This is done. > > > 2. usr/src/lib/Routes/Makefile.sfw > Lines 45-46, why are you uncompressing it into a tmp dir > and then moving it? Couldn't it have been uncompressed > directly into $(VER) dir? Or even better could you have used the > now inbuilt mechanism for unpacking the src tarball? This is done. > > 3. usr/src/lib/Routes/sunman/py-routes.3 > Typo on line 4 > > >>> The SUNW Package name is wrong in various places > >>> > >>> Its not normal to give the full url to the source in > >>> a man page - see other examples in gate > > - these are still wrong Corrected. > > 3A. usr/src/lib/Makefile > & usr/src/pkgdefs/Makefile > Changes to these are not in the webrev! > Included. > 4. usr/src/lib/Routes2.4-bak > What is this stuff in the webrev? > > Removed. >>>> 9. usr/src/pkgdefs/SUNWpython24-routes/depend & 26 >>>> This also needs all the core pkgs in it, ie. those in the >>>> default depend file. > > - this is still wrong > Included all the dependencies. > >>>> 10. usr/src/pkgdefs/SUNWpython24-routes/pkginfo.tmpl & 26 >>>> Add the pkg version to the end of the DESC= line, eg... >>>> DESC="............. (1.10.3) > > - put the version at the end of the line as above > Done. > 11. usr/src/pkgdefs/SUNWpython26-routes/prototype_i386 >>>> SUNW pkg name is wrong > Corrected. > 12. SUNWpython24-routes/prototype_com > & SUNWpython26-routes/prototype_com > & Routes/Makefile.sfw > I still don't like that you are delivering the same files > twice in two different packages. > Just a thought: Could one be a symbolic link > to the others files, ie. the 24 files be a symbolic link to > the 26 file, the symbolic links being set up in > SUNWpython24-routes/prototype_com and 24 depend on 26 ? > The only files which are common in SUNWpython24-routes and SUNWpython26-routes is Makefile and copyright file. I thought it is better to have separate Makefile rather than have a link. -manjunath > END
