[Bug 438588] Review Request: zfstream - C++ iostream like access to compressed files

bugzilla at redhat.com bugzilla at redhat.com
Thu Apr 30 13:45:13 UTC 2009


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=438588





--- Comment #13 from Thomas Sailer <t.sailer at alumni.ethz.ch>  2009-04-30 09:45:09 EDT ---
Update
Spec URL: http://sailer.fedorapeople.org/zfstream.spec
SRPM URL: http://sailer.fedorapeople.org/zfstream-20041202-3.fc11.src.rpm

(In reply to comment #12)
> The file ChangeLog can go to %doc. I don't think it is worth packaging the
> LICENSE file as it doesn't say much about the license.

Done and agreed.

> * Patches should be explained and be submitted to upstream. Since the project
> seems dead, it won't hurt to bypass the submission part. But please explain in
> the SPEC file what the patch does.

I have sent the patch upstream by PM (the project lacks a bug tracker). While
the original author said he integrated the patch into his tree, he apparently
hasn't gotten around to release a new version.

> * Similarly, please give the reasoning of Source1.

See comment #9. Basically because I couldn't get toc working, and furthermore
allows autotools to cross-compile the lib (I have also submitted
mingw32-zfstream for review).

> * Please remove the duplicate copy of minizip, which we already have in Fedora,
> from Source1 and adjust the BR's. Note that minizip-devel already requires
> zlib-devel.

Done.

> * The package must be named according to the Package Naming Guidelines. The
> upstream tarball has a different name. Why don't we use it?

The project is named just "zfstream" on the author's homepage. It's just that
the tarball has libs11n_ prefixed. s11n.net is another project by the same
author, so I guess that's where the prefix comes from. zfstream however has
nothing to do with s11n.net, so I found it more natural to just name the
package zfstream.

> * No need to explicitly BR automake. libtool does and always will pull that for
> you.

Done

> ! Please make use of the %{name} macro.

Done.

> * The devel package must require "bzip2-devel", "zlib-devel or minizip-devel
> (?)", and "pkgconfig" 

Done. I think zlib-devel is enough, as zipstream.hpp does not include anything
from minizip-devel, minizip is only needed for building

> ? About "touch NEWS README AUTHORS": Wasn't there an --add-missing flag to
> automake?

Actually, -a (which is already present) or --add-missing (the full name for -a)
only adds missing files _other_ than NEWS, README and AUTHORS. So build will
fail if you remove the touch.

> ! No need for blank %doc's.

Done.

> ! Please make the descriptions span 80 columns.  

Shortened.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.




More information about the package-review mailing list