On Mon, 16 Feb 2004, Jari Aalto+mail.linux wrote: > * 2004-02-13 Daniel Reed <nmlorg <AT> cygwin.com> list.cygwin-apps > > | Package: joe 2.9.8-1 [2003-11-11] > | Description: Fast and simple editor which emulates 5 other editors > | [snip] > | Good to go: Gerrit P. Haase > | Status: Attained required 3 votes. Package available. Reviewed. > | HOLD-UPS: > > repackaged > > mkdir boxes ; cd boxes > wget -q -O - http://tierra.dyndns.org:81/cygwin/boxes/get.sh | sh
BTW, should this be "s/boxes/joe/g"? > | Package: boxes 2000.0401-1 [2004-01-29] > | Description: Text filter which can draw any kind of ASCII art box around its input > text. > | Proposer: Jari Aalto > | Proposal: http://cygwin.com/ml/cygwin-apps/2004-01/msg00246.html > | http://tierra.dyndns.org:81/cygwin/boxes/boxes-2000.0401-1.tar.bz2 > | http://tierra.dyndns.org:81/cygwin/boxes/boxes-2000.0401-1-src.tar.bz2 > | http://tierra.dyndns.org:81/cygwin/boxes/setup.hint > | Problems: 21943873id2594723 The README reports the package version as 1.0.1. > | 21943873id2594728 Running the boxes.exe executable results in a > "boxes: Can't find config file." message. Looks like it expects the config file in > "/usr/local/share/boxes". > | 21943873id2594734 The man page has a --GLOBALCONF-- string for the > system-wide config file name -- should that have been replaced by something like > "/usr/share/boxes/boxes.cfg"? > | 21943873id2594742 Source problems: See cygwin-apps-get.13000 > | Status: Attained required 3 votes. Package available. > | HOLD-UPS: Unresolved problems. No "good to go" review. > > Problems fixed and repackaged. > > mkdir boxes ; cd boxes > wget -q -O - http://tierra.dyndns.org:81/cygwin/boxes/get.sh | sh Ok, here's a second review. Problems (and nits) outlined below: setup.hint: 1) ldesc has leftover hyphenation ("pro- gramming"), and is all on one line. Binary package: 2) The man page reports "This is boxes version --BVERSION--.", and still has the --GLOBALCONF-- string. Something wrong with macro substitution, perhaps? 3) The Cygwin-specific readme still indicates 1.3.22 as the minimum Cygwin version. Otherwise the binary package looks good. Now for the source package: -- patch 4) The patch contains boxes.README and boxes.README.b (the latter being the old version of the README, AFAICS). 5) The patch to the Makefile changes the value of GLOBALCONF, but there's a build.sh script in the patch that explicitly passes GLOBALCONF to make. That's overkill, IMO -- the build.sh script isn't necessary for this. 6) Is there a reason you remove boxes.1 (a generated file) via patch? This is where build.sh can come in handy (and make the patch smaller ;-). 7) There's a typo in the patch for src/Makefile: "+exec_prefix = $(refix)". Frankly, I don't see how this ever built correctly... -- script 8) The script will put the build files in /usr/src/cygwin-packages. I do NOT want any script to muck with my /usr/src unless I run it from there. The package build should be contained under the current directory. 9) The build produces some warnings (mostly "implicit declaration" -- missing headers?). 10) Looking at the output, the script excludes generated files from the patch, but, apparently, doesn't pick up boxes.1 as a generated file... This could explain 6) above. 11) The usr/share/man, usr/share/doc/boxes-20000401, and usr/share/boxes directories (and their subdirectories) end up as non-executable in the binary tarball created by the script (wrong flags to install?). 12) The build doesn't happen in boxes-20000401/.build, it happens directly in boxes-20000401... 13) The script leaves the boxes-20000401 directory behind... Don't know if it should be expected to clean it up, though... Some of the above are nits, but some (like the build script using /usr/src) are serious. Igor -- http://cs.nyu.edu/~pechtcha/ |\ _,,,---,,_ [EMAIL PROTECTED] ZZZzz /,`.-'`' -. ;-;;,_ [EMAIL PROTECTED] |,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski, Ph.D. '---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow! "I have since come to realize that being between your mentor and his route to the bathroom is a major career booster." -- Patrick Naughton
