Prashant Srinivasan wrote:
> Hi Amanda,
> Some more comments for you:
> .gem_mirror should have system("gem", "mirror", *pass_on_args) instead
> of *ARGV
>
You're right, so much for my QA process.
> .Will it help if the gem version were explicitly mentioned in the gem
> man page?
>
No, users should use gem env to find out that information. Besides at
the moment they are still able to update rubygems and overwrite the
package installed version. So they may not actually have the version we
are integrating.
> .typo in gemlock, "depracation" for deprecation
>
Thought I go all those, although I think you're having as much trouble
spelling as I am :o)
> .in gemri, do we need to pen a note telling users that the program will
> execute in 5 seconds?
>
No, they should use 'ri' instead, I'll reduce it to 3 seconds. It does
say to use 'ri' and also mentions that the warnings can be suppressed
with -s.
I'll wait to hear if you have any more comments before updating the
WebRev. The OSR got lost in the post so I can't do the putback this week.
Thanks for the review
Amanda
> -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
>>
>>
>
> _______________________________________________
>
>
> webstack-discuss mailing list
> webstack-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/webstack-discuss
>
