Vasilisa, I didn't see anyone review this for you, so see below for my comments from my *quick skip* through ...
Paul Vasilisa Sidorova wrote: > > I'm working on upgrading svn from version 1.4.3 to 1.5.2 > Please help to review the code. > > The webrev is at, > http://cr.opensolaris.org/~vs158390/svn.webrev/ === Start of Comments === 1. usr/src/cmd/subversion/METADATA Cosmetic: Align the fields better 2. CDDL HEADER and top of files (various files) Cosmetic: Change the tops of the files so that they conform to those in ... "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; See http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines mainly just missing line space after the CDDL header. 3. usr/src/cmd/subversion/Makefile.sfw Change so that it uses the predefined './configure --prefix=..' value from Makefile.master, ie. $(CFGPREFIX), but you could also change this configure bit so it uses the CONFIGURE_OPTIONS method, eg. something like .... CONFIGURE_OPTIONS += --with-apxs=..... CONFIGURE_OPTIONS += --with-ssl CONFIGURE_OPTIONS += --with-zlib=/usr CONFIGURE_OPTIONS += --with-jdk=/usr/java etc. ... ... ./configure $(CONFIGURE_OPTIONS) Does it really need the following ? ... 184 chmod -R a+r $(VER) 185 find $(VER) -type d -exec chmod a+x {} \; if it does change the \; to \+ (its faster) Also does it need ? ... 54 @find . -name core -exec rm -f {} \; if so why? 4. Copyright year wrong (various files) The Copyright year needs changing in various files that you have updated (to 2009) 5. usr/src/pkgdefs/SUNWsvn/prototype_com Install and new files into /usr without the write permission bit set. === End of Comments ===== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
