Evgeny,
I took another quick look ....
>> 3. usr/src/cmd/jruby/install-jruby
>> You don't need the 'mkdir -p ..' stuff as the dir
>> is already in Targetdirs.
Doesn't look as though you changed this
>> 8. usr/src/pkgdefs/SUNWjruby/pkginfo.tmpl
>> Some people put the pkg version at the end of the
>> DESC= line (feel free to ignore this)
I think you probably misunderstood what I meant; I don't mean 'move the
DESC= line to the end of the file', what I meant was 'add something like
the following to that line' ...
DESC="Java implementation of the Ruby programming language (1.1.3)"
and it's probably better to move the line back to where it was (rather
than it the end of the file).
>> 9. usr/src/pkgdefs/SUNWjruby/prototype_com
>> Do all these files really need the write permissions
>> bit set?
What was your conclusion on this?
The rest looks okay to me
Paul
Evgeny Bessonov wrote:
> Hi Paul,
>
> thank you for your review!
>
> Code was fixed according to your/ Roland's comments
> with exception of p3.3 (cp vs _install).
> I prefer to stay with cp because a lot of
> stuff (11668 files/ 1858 folders) should be copied.
> I've updated webrev page:
>
> http://cr.opensolaris.org/~jinb/jruby2
>
> Is it better now?
>
> Thanks a lot.
> With best regards
> Evgeny
>
>
> Paul Cunningham wrote:
>> Evgeny,
>>
>> See below for my comments on this ...
>>
>> Paul
>>
>> Evgeny Bessonov wrote:
>>
>>>
>>> I'm working on poring JRuby (Java implementation
>>> of the Ruby language) 1.1.3 into OpenSolaris.
>>> And here is webrev for jruby/SUNWjruby:
>>> http://cr.opensolaris.org/~jinb/jruby1/
>>>
>>> We have not enough time till svn_99/100 gate closing
>>> so could you please review/ approve/ provide your feedback/notes ASAP?
>>
>>
>> === Start of Comments ====
>>
>> 1. usr/src/cmd/jruby/METADATA
>> Add line to say where the source code came from 'URL:...'
>>
>> 2. usr/src/cmd/jruby/Makefile.sfw
>> You also might want to use 'env - ' rather than 'env '
>>
>> 3. usr/src/cmd/jruby/install-jruby
>> You might want to
>> look at 'Roland Mainz' comments on other people's
>> install-* scripts, re the shell used and . command,
>> and apply those comments - look back through
>> sfwnv-discuss for them.
>>
>> You don't need the 'mkdir -p ..' stuff as the dir
>> is already in Targetdirs.
>>
>> Do the files copied into proto with 'cp' need their
>> permissions set? Why not do it with '_install N ...'.
>>
>> 4. usr/src/cmd/jruby/test
>> Where is this used ?
>>
>> 5. usr/src/pkgdefs/SUNWjruby/Makefile
>> Copyright year is wrong?
>>
>> You could remove the null DATAFILES= line
>>
>> 6. usr/src/pkgdefs/SUNWjruby/depend
>> Copyright year is wrong?
>>
>> Move copyright lines to after 'CDDL HEADER END'
>> header.
>>
>> 7. usr/src/pkgdefs/SUNWjruby/pkginfo
>> You don't need this checked-in as it is auto
>> generated from SUNWjruby/pkginfo.tmpl
>>
>> 8. usr/src/pkgdefs/SUNWjruby/pkginfo.tmpl
>> Some people put the pkg version at the end of the
>> DESC= line (feel free to ignore this)
>>
>> 9. usr/src/pkgdefs/SUNWjruby/prototype_com
>> Do all these files really need the write permissions
>> bit set?
>>
>> Format and year of the copyright lines are wrong
>>
>> 10. usr/src/pkgdefs/SUNWjruby/prototype_i386
>> & usr/src/pkgdefs/SUNWjruby/prototype_sparc
>> Copyright year is wrong?
>>
>> === End of Comments ======
>>
--
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products
