Okay, looks good to me Paul
rusong.zheng wrote: > > Thanks for your comments. Please my inline comments. > > Paul Cunningham wrote: >> Rusong, >> >> This mainly looks good to me, see below for a few minor comments ... >> >> Paul >> >> rusong.zheng wrote: >>> >>> 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/ >> >>> Rusong.Zheng wrote: >> >>>> I'm porting jedit, a programmer's text editor written in Java. >>>> >>>> Please help to code review my modification. The webrev is at: >>>> http://cr.opensolaris.org/~rszheng/jedit/ >> >> 1. usr/src/Targetdirs >> & usr/src/cmd/Makefile >> & usr/src/pkgdefs/Makefile >> These look as though they need resyncing with the gate/clone. > Done >> >> 2. usr/src/cmd/jedit/Makefile.sfw >> Line ... >> 61 -rm -rf $(JEDIT_SRC_DIR) $(JEDIT_NOT_USE) jedit >> should it be removing $(JEDIT_NOT_USE) and jedit from >> usr/src/cmd/jedit ? (is it because the build bit puts stuff >> into $(JEDIT_SRC_DIR)/.. rather than containing it in >> $(JEDIT_SRC_DIR)) >> >> END > yes, it is. > source package of jedit downloaded from website is a lit strange. It named > jedit4.3pre16source.tar.bz2* > *When bunzip2 and untar, it will get > build-support/ > jEdit/ > > build-support is not used when building jedit, which I defined to > JEDIT_NOT_USE > jEdit is what I defined to JEDIT_SRC_DIR, and all stuff needed are in it. > > jedit is a script I add to run jedit in commandline, it does not exist > before building and should be removed when cleaning -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit General Dynamics Itronix Europe Ltd.
