This looks okay to me Paul
Amanda Waite wrote: > Thanks for the review. Here's the updated webrev: > > http://cr.opensolaris.org/~tekgrrl/gems131-CR6764580-2/ > > Also see comments inline. > > Paul Cunningham wrote: >> === Start of Comments ==== >> >> 1. usr/src/Targetdirs >> As you have taken all the other stuff out of here, why not >> take those out too (if no other pkg is likely to use them) >> that way you wont have to update Targetdirs for the next >> version update ... >> 1094 /var/ruby/1.8/gem_home/doc/rubygems-1.3.1 \ >> 1095 /var/ruby/1.8/gem_home/doc/rubygems-1.3.1/ri \ >> 1096 /var/ruby/1.8/gem_home/doc/rubygems-1.3.1/rdoc \ > > Done > >> >> 2. usr/src/cmd/ruby18/METADATA >> Add NAME: field. > > Done >> >> 3. usr/src/pkgdefs/SUNWruby18r/Makefile >> & usr/src/pkgdefs/SUNWruby18r/prototype_i386 >> & usr/src/pkgdefs/SUNWruby18r/prototype_sparc >> & usr/src/pkgdefs/SUNWruby18u/Makefile >> I think you have only updated the copyright year in this file, >> so you didn't really need to change it. > > The ident whitespace was all wrong so I fixed it. >> >> 4. usr/src/pkgdefs/SUNWruby18r/copyright >> Have you actually changed this? webrev shows no diffs > > Reset to the parent version > >> >> 5. usr/src/pkgdefs/SUNWruby18r/prototype_com >> Do the files really need the write permission bit set? >> >> Why does it install 'doc' files in /var ?? > > Ruby doc files are dynamically generated and all RubyGems docs live in > directory specified by GEM_HOME. These files are read by the rdoc and ri > commands >> >> 6. usr/src/pkgdefs/SUNWruby18u/depend >> I can't see what you have changed in here. I think you >> have changed something because the year is 2009 by webrev >> is not showing it > > Reset to the parent version > >> >> 7. usr/src/pkgdefs/SUNWruby18u/pkginfo.tmpl >> The DESC: line should probably be more descriptive > > Done > >> >> 8. usr/src/pkgdefs/SUNWruby18u/prototype_com >> Do the files really need the write permission bit set? > > No, but the perms haven't changed in this update and no new files have > been added. I changed them anyway but had to use chmod as protofix > doesn't like the parameterized paths in the prototype_${MACH}.tmpl files > >> >> 9. usr/src/pkgdefs/SUNWruby18u/prototype_i386.tmpl >> & usr/src/pkgdefs/SUNWruby18u/prototype_sparc.tmpl >> I think you have only updated the copyright year in this file, >> so you didn't really need to change it. > > See 3 >> >> And why are these .tmpl files? > > The paths in the .tmpl files are parameterized and the real files are > created by the build process. > >> >> 10. man *.1 files >> I haven't checked the text in these >> >> But is it normal to have this line ... >> "Source code for RubyGems is available on >> http://rubyforge.org/projects/rubygems/"; >> it may not be in n years time for the version you >> are delivering. > > Changed > >> >> 11. ruby scripts >> I've only flicked through these as I don't know ruby > > We'll have to rely on one of my team to review the Ruby stuff then. >> >> === End of Comments ====== > > > -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit General Dynamics Itronix Europe Ltd. Pioneer House Chivers Way Histon, Cambridgeshire, UK, CB24 9NL Ph: +44 (0)1223 200648 FAX: +44 870 4324162 Email: paul.cunningham at tadpole.com This email message is for the sole use of the intended recipient(s) and may contain GDC4S confidential or privileged information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message
