Hi Paul, >> 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
But prev. line(36) contains rm ?rf the-same-dir, so it should be recreated after possible removal (mkdir ?p on next line 37). >> 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). Yes. Sure. It's my fault. I'll add version and move DESC back. >> 9. usr/src/pkgdefs/SUNWjruby/prototype_com >> Do all these files really need the write permissions >> bit set? > What was your conclusion on this? As I see in packages tree there are a lot of examples of 755 for folders/ runable stuff and 644 permissions set for the rest. Thanks a lot. With best regards Evgeny Paul Cunningham wrote: > 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 ====== >>> >
