Shweta, See below for my comments ...
Paul >> shweta phabba wrote: >>> >>> I have created the webrev for package sqlalchemy. please review the >>> code for sqlalchemy porting >>> Link for sqlalchemy webrev is , >>> http://cr.opensolaris.org/~pshweta/sqlalchemy/ == START == 1. usr/src/Targetdirs It doesn't look as though you have changed this, if not remove it from your checked in stuff. 2. usr/src/lib/Makefile Add your stuff alphabetically 3. usr/src/lib/sqlalchemy/METADATA PROJECT_URL: should probably be "http://www.sqlalchemy.org/"; and SOURCE_DOWNLOAD: should probably be something like ... "http://sourceforge.net/projects/sqlalchemy/files/sqlalchemy/0.5.5/SQLAlchemy-0.5.5.tar.gz/download"; Delete "Any additional information that you want to supply goes down here" text on the COMMENTS: line. Change "NAME: sqlalchemy" to SQLAlchemy so you can use it in Makefile.sfw (see 4) 4. usr/src/lib/sqlalchemy/Makefile.sfw Extract the VER= info from the METADATA, see other pkgs in gate for how to do this. 5. usr/src/lib/sqlalchemy/install-sfw Pass the SQLAlchemy-0.5.5 version info in from your Makefile.sfw so you don't have to keep changing it every update; again see gate for examples. I think you can delete lines 43 to 49 Should there be a man page for this pkg, you may need to create your own for it ? 6. usr/src/pkgdefs/Makefile Where are the changes to this to add SUNWsqlalchemy into the list 7. usr/src/pkgdefs/SUNWsqlalchemy/Makefile Copyright year wrong - also check all other files as there are others wrong to Delete 'DATAFILES= depend' as you have your own 'depend' file 8. usr/src/pkgdefs/SUNWsqlalchemy/copyright Is this the full licence text, if not you need to add it. You need to add the copyright info for the source owner(s). 9. usr/src/pkgdefs/SUNWsqlalchemy/depend Move the Copyright lines down to after the CDDL HEADER END and correct the year. The SCCS ident info line doesn't look right, check it and all other files to as other are wrong. 10. usr/src/pkgdefs/SUNWsqlalchemy/pkginfo This files shouldn't be in the webrev as its autogenerated from the SUNWsqlalchemy/pkginfo.tmpl file as build time 11. usr/src/pkgdefs/SUNWsqlalchemy/prototype_com The top of the file 'CDDL' header, etc, does not conform to the prototypes, see guide lines in http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines 11. How How is this invoked, should there be something in /usr/bin ? == END ===
