Ru-Song Vincent Zheng wrote: > Hi Mark, > > Thanks for your review. Please see my inline comments. > > ----- ??? ----- > ?: Mark Martin <storycrafter at gmail.com> > ??: ???, 2009 ? 6? 18?, ??11:57 > ??: Re: [sfwnv-discuss] Code review request for jedit after PSARC > ???: "rusong.zheng" <Rusong.Zheng at Sun.COM> > ??: sfwnv-discuss at opensolaris.org > > >> rusong.zheng wrote: >> >>> Hi, >>> >>> Per ARC's request, changes to webrev are : >>> >>> /usr/share/jedit/4.3pre16/ ==>/usr/share/jedit/ >>> >>> sunman/jedit.1 ==>jedit.1 sunman is >>> >> removed>build.xml.patch >> ==>patches/build.xml.patch>build.properties.patch >> ==>patches/build.properties.patch>jedit.patch >> ==>patches/jedit.patch >> >>> directory classes is removed for that contents of it are >>> >> duplicated (class files and API docs, class files already be >> contained in jedit.jar, APIs doc already in doc/API/) >> >>> jedit is removed from /usr/share/jedit/jedit and moved to >>> >> /usr/bin/jedit> >> >>> Makefile.sfw, install_sfw and prototype_com are changed accordingly. >>> >>> Besides that, METADATA file has been changed to meet the new >>> >> requirement. All the others are the same as the last round of review. >> >>> Please help review them. >>> >>> Webrev is at: >>> http://cr.opensolaris.org/~rszheng/jedit/ >>> >>> PSARC case url is: >>> http://arc.opensolaris.org/caselog/PSARC/2009/357/ >>> >> I'm confused about the script "jedit", and I'll admit it's because >> this is the first java->sfw project I've code reviewed. That, and >> my Make is probably a little rusty. >> >> It occurs to me that the patch creates a file jedit.sh. If that is >> true:a) Where is "jedit" that's been referenced as /usr/bin/jedit >> b) Is that the appropriate name? >> >> >> > Yes, jedit here is a script to run "java -jar jedit.jar". > for a), the script did not exist firstly. I wrote a script and make a diff to > generate the patch, and original script I wrote is removed. When build is > finished, the script will be generated and installed to /usr/bin/. So > /usr/bin/jedit is itself a script and don't refer to other file. > > for b), I think it is easy to use: user can just type "jedit" commandline to > run jedit.jar. >
You're right, of course. I misinterpreted what I read from the patch. Too rusty. Looks fine.
