Chris, See below for my comments, some of them may be the same as SteveC's but I have included them again anyway ...
Paul On Fri, 2008-07-11 at 10:15, Chris Liu wrote: > Hi all, > > Please help code review the inclusion of mtx-1.3.11 into the sfw > consolidation. It is a media changer controller software. > > The webrev is at: > http://cr.opensolaris.org/~chrisliu/MTX > === Start of Comments ==== 1. usr/src/cmd/Makefile & usr/src/pkgdefs/SUNWmtx/Makefile This needs to be resync'ed with the gate so it doesn't look as though you are trying to delete stuff. 2. usr/src/cmd/mtx/METADATA You need to add all the missing stuff, package name, etc 3. usr/src/cmd/mtx/Makefile.sfw Use the predefined '--prefix' value from Makefile.master, eg. use .. ./configure $(CONFIGURE_OPTIONS) Does the community provide a bz2 version of the tarball? If so use that instead of the gz one - it is smaller. 4. usr/src/cmd/mtx/install-sfw You don't need the 'mkdir -p' as those directories should be in Targetdirs and so already created. 5. usr/src/pkgdefs/SUNWmtx/depend This looks like the default 'depend' file, so you can delete this and add 'DATAFILES = depend' into the Makefile instead. That's assuming there are no missing dependencies - have you run the dependency checker script on the package? 6. usr/src/pkgdefs/SUNWmtx/pkginfo.tmpl I agree with SteveC that the pkg's version should be on the DESC= line. But loads of people don't do this and there is also a number of variants of how it it done. So I guess it's up to you if there is no rule :-( (The excuses for not putting it in are: I might forget to update it when the package gets reved. Also, it doesn't change if the pkg is patched to a later rev. And it goes away anyway with IPS). === End of Comments ======
