Bill, Here are a few comments from my quick skip through, see below ...
Paul Yan Xue Yang wrote: > Hi Folks, > > Please help review the fix of CR 6689922. Any comments are welcome. Thanks. > > Webrev is at: > > http://cr.opensolaris.org/~billyan/unison/ > > CR synopsis: > > 6689922 unison 2.27.57 to be included into SFW consolidation > > PSARC case: > > http://opensolaris.org/os/community/arc/caselog/2008/212/ === Start of comment ==== 1. usr/src/cmd/unison/METADATA Doesn't have a 'PACKAGE:' field. Are the other field names supposed to be in upper case? 2. usr/src/cmd/unison/Makefile.sfw Should the ocaml stuff be delivered as a separate package builds (maybe)? You seem to be building and installing parts of this in the configure stage (so again split into separate pkgs maybe) 3. usr/src/cmd/unison/install-unison The Copyright' year is wrong There are unused path defines in here (remove) 4. usr/src/cmd/unison/unison.1 Could this have been patched from the file in the tarball? 5. usr/src/pkgdefs/SUNWunison Does the package need a 'depend' file or does it need to use the default depend? 6. usr/src/pkgdefs/SUNWunison/prototype_i386 + usr/src/pkgdefs/SUNWunison/prototype_sparc Move copyright line to after 'CDDL HEADER END'. It is also not complete (2 lines) 7. usr/src/pkgdefs/SUNWunison/prototype_com Are there any other bits that need to be packaged (or is the ocaml libs hard linked into unison)? === End of Comments ===== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Computer Products
