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 ====== >
