Bug 432080 - Review Request: cstream - General-purpose stream-handling tool
Summary: Review Request: cstream - General-purpose stream-handling tool
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-02-08 18:58 UTC by Hans Ulrich Niedermann
Modified: 2008-02-11 23:05 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-02-11 23:05:11 UTC
Type: ---
Embargoed:
panemade: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Hans Ulrich Niedermann 2008-02-08 18:58:43 UTC
Spec URL: http://ndim.fedorapeople.org/cstream/2.7.4-3/cstream.spec
SRPM URL: http://ndim.fedorapeople.org/cstream/2.7.4-3/cstream-2.7.4-3.src.rpm
Description: General-purpose stream-handling tool

cstream filters data streams, much like the UNIX tool dd(1).

It has a more traditional commandline syntax, support for precise
bandwidth limiting and reporting and support for FIFOs.

Data limits and throughput rate calculation will work for files > 4 GB.

Comment 1 Parag AN(पराग) 2008-02-09 09:43:50 UTC
1)you may like to add distag to release as
Release:  3%{?dist}

2)good if you add tests to %doc that will create rpmlint warning to silent it
add to %prep
chmod 644 tests/test1.sh


3) CFLAGS value is 
 CFLAGS='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables'

  So you don't need to add -Wall. Also, any reason to add 
"-Wextra -Wno-unused-parameter -Werror" to make install?

Comment 2 Hans Ulrich Niedermann 2008-02-09 22:10:40 UTC
1) Of course. Added.

2) The Makefile.am remarks that the tests do not work (due to the output format
being slightly different). I'd presume running them at %check time or shipping
them as docs does not make sense, then.

3) Can a double -Wall hurt? Adding "-Wall -Wextra -Werror" makes it clear which
set of warnings/errors we want, so that's what I'd go for.

   Compilation should be done before "make install", so I do not deem it
necessary to define the CFLAGS there.

Updated package at http://ndim.fedorapeople.org/cstream/2.7.4-4.fc8/

* Sat Feb 09 2008 Hans Ulrich Niedermann <hun> - 2.7.4-4
- Add %%{?dist} to Release:

* Fri Feb 08 2008 Hans Ulrich Niedermann <hun> - 2.7.4-3
- More compile warnings (-Wall -Wextra -Werror).
- Redacted description down to the most important points.


Comment 3 Parag AN(पराग) 2008-02-11 06:09:48 UTC
Review:
+ package builds in mock (rawhide i386).
koji build => http://koji.fedoraproject.org/koji/taskinfo?taskID=413928
+ rpmlint is silent for SRPM and for RPM.
+ source files match upstream.
eab4c98afef79766dd61a6a36dec845a  cstream-2.7.4.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc files present.
+ BuildRequires are proper.
+ Compiler flags are honored correctly.
+ defattr usage is correct.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ no .pc file present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets are used.
+ Not a GUI app.

 SHOULD:
   Though I am not happy with extra flags to compiler just because it gives
extra information, its ok to add them here.

APPROVED.

Comment 4 Hans Ulrich Niedermann 2008-02-11 10:41:05 UTC
New Package CVS Request
=======================
Package Name: cstream
Short Description: General-purpose stream-handling tool
Owners: ndim
Branches: F-7 F-8
InitialCC: 
Cvsextras Commits: Yes


Comment 5 Kevin Fenzi 2008-02-11 17:56:51 UTC
cvs done.

Comment 6 Hans Ulrich Niedermann 2008-02-11 23:05:11 UTC
cstream-2.7.4-4 packages are in rawhide and the update queue for F-7, F-8.

Thanks for review and CVS.



Note You need to log in before you can comment on or make changes to this bug.