This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Review: no go] script-20041106-1


On Sat, 6 Nov 2004, Andrey Butov wrote:

> I want to package and maintain the 'script' utility
> for Cygwin. This is a cygwin implementation of the
> UNIX script utility, which can be used to record a
> transcript of all activity in the terminal session.
> This includes a recording of all inputs and outputs.
> The results are stored in a file which is called
> 'typescript' by dfault.
>
> http://www.angelfire.com/ab8/abutov/setup.hint
> http://www.angelfire.com/ab8/abutov/script-20041106-1.tar.bz2
> http://www.angelfire.com/ab8/abutov/script-20041106-1-src.tar.bz2

Andrey,

Cygwin packages don't automatically get uploaded -- they have to first get
voted on and reviewed by other maintainers.  You need five votes, and all
those who decide to review have to give a "Good-to-go" (GTG) before the
package gets uploaded.  Some packages, notably ports of well-known
utilities included in Linux distros, are exempt from the voting
requirement.

Here's one review (though not a GTG one).

Disclaimer: I haven't actually tested the binary or looked in depth at the
C source file, so the comments below are based solely on packaging.

The binary package contains no documentation whatsoever -- no man page, no
READMEs, and the help option leaves much to be desired (i.e., you only get
help if you give wrong arguments to the executable).  The Cygwin-specific
README contains no information about the canonical project page, version,
or development history, but does include the Makefile (from the source
package) verbatim.

I couldn't determine whether the executable is stripped -- if it isn't, it
needs to be.

Oh, and a couple of minor nits: directories in the tarballs start with ./
-- this is annoying; and please don't put comments in setup.hint -- they
belong in the ITP message or the Cygwin-specific README.

I suggest changing the Cygwin-specific README to include at least the
information from setup.hint, adding a manpage, adding a --help option to
script.exe, repackaging the tarballs properly, and adding an "install"
target to the Makefile that strips the executable and moves it to the
right place in the tree (e.g., $DESTDIR$prefix/bin).  Once those are
fixed, and someone actually tests the executable, we can consider
uploading this.

BTW, regarding CGF's comment that this is automatically accepted: this is
*NOT* a port of "script" from util-linux, this is a completely new
implementation, and as such, IMO, needs to be voted on.
	Igor
-- 
				http://cs.nyu.edu/~pechtcha/
      |\      _,,,---,,_		pechtcha@cs.nyu.edu
ZZZzz /,`.-'`'    -.  ;-;;,_		igor@watson.ibm.com
     |,4-  ) )-,_. ,\ (  `'-'		Igor Pechtchanski, Ph.D.
    '---''(_/--'  `-'\_) fL	a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"The Sun will pass between the Earth and the Moon tonight for a total
Lunar eclipse..." -- WCBS Radio Newsbrief, Oct 27 2004, 12:01 pm EDT


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]