Rusong, Additional comments below ...
Paul 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/ ... cut ... 1. usr/src/cmd/jedit/Makefile.sfw Line ... 59 -rm -rf $(JUNIT) is JUNIT correct ? > I also added two new variables PROG_NAME and PROG_VER to hold jedit name > and version, which will be used in install.sfw. You might want to explicitly pass those into install.sfw so that it is clearer that is what is happening. 2. usr/src/cmd/jedit/install-sfw Line ... 69 cd ${TOP}/jEdit/build you could use the following instead I think .. cd ${TOP}/${JEDIT_SRC_DIR}/build Line .. 92 _install N ../../jedit ${BINDIR}/jedit 555 could this be a symbolic link instead to the file installed in .. 91 _install N ../../jedit ${JEDIT_ROOT}/jedit 555 Also, is jedit a binary or a script, if so you should probably use either '_install E ' or '_install S ' 3. usr/src/cmd/jedit/build.xml.patch Observation: then file that this is patching has .. debug="true" switched on, do you really want that ? 4. usr/src/cmd/jedit/jedit.patch The 'read permissions' are no set correctly on this file, I can not see it. 5. usr/src/pkgdefs/Makefile & usr/src/cmd/Makefile & usr/src/Targetdirs These files need resyncing with the gate so it does look as though you are trying to change other stuff 6. dependencies You install a jar file so doesn't it have a dependency on the java runtime stuff ? > Though the jre > is needed to run jedit ( but for solaris/opensolaris, the jre is always > installed with the fresh system) but if i were to remove jre then jedit wouldn't work, so without the dependency there is nothing to stop me removing jre without a warning! -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
