Hi Amanda,
Some more comments for you:
.gem_mirror should have system("gem", "mirror", *pass_on_args) instead
of *ARGV
.Will it help if the gem version were explicitly mentioned in the gem
man page?
.typo in gemlock, "depracation" for deprecation
.in gemri, do we need to pen a note telling users that the program will
execute in 5 seconds?
-ps
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 ======
>>
>
>
>
> _______________________________________________
> sfwnv-discuss mailing list
> sfwnv-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
>
