Bug 183953 - Review Request: bit (A bit-oriented data stream parser and gtkmm widget set)
Review Request: bit (A bit-oriented data stream parser and gtkmm widget set)
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael Schwendt
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT bitgtkmm
  Show dependency treegraph
 
Reported: 2006-03-03 20:32 EST by Rick L Vinyard Jr
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-07-07 11:58:16 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
failed test output (1.02 KB, text/plain)
2006-03-27 18:26 EST, Michael Schwendt
no flags Details
bit unit test results (2.95 KB, text/plain)
2006-05-17 00:39 EDT, Rick L Vinyard Jr
no flags Details

  None (edit)
Description Rick L Vinyard Jr 2006-03-03 20:32:38 EST
Need sponsor.

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/bit.spec
SRPM Name or Url: http://miskatonic.cs.nmsu.edu/pub/fedora/4/srpms/bit-0.1.26-1.src.rpm

Description:
The purpose of the bit library is to simplify the parsing of data streams into fields that are not octet (byte) oriented. The library also contains a collection of gtkmm widgets for display of the parsed data streams.

Other notes:
This library is dependent upon another library (conexus) that is pending review.
Comment 1 Peter Gordon 2006-03-03 20:53:42 EST
Not a review, but a quick comment (as this hit me when I first submitted
lucidlife for review): You don't really need the Requires: line in the main
package, as that is redundant (the BuildRequires: *-devel things will pull in
the .so name dependencies).
Comment 2 Rick L Vinyard Jr 2006-03-04 04:35:04 EST
Thanks. I had some problems with another package I've submitted, and made a new
release. But, I made the changes before seeing this, so it still has the
Requires and BuildRequires for now.

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/bit.spec
SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/4/srpms/bit-0.1.27-1.src.rpm
Comment 3 Jonathan Underwood 2006-03-05 08:15:38 EST
Just a few comments, as I am still awaiting a sponsor, so cannot do a full
review, but:

1) Source: http://prdownloads.sourceforge.net/libbit/bit-0.1.27.tar.gz 
can be replaced with  
Source: http://prdownloads.sourceforge.net/libbit/%{name}-%{version}.tar.gz

2) the "-n bit-0.1.27" seems to be redundant in the %prep section


Comment 4 Rick L Vinyard Jr 2006-03-05 13:14:13 EST
Right on both counts. When I first created the .spec file I wasn't sure if I was
going to need to vary the package name from the distribution name, so I make
them separate. Since I don't need to, I can eliminate that bit of cruft.

Thanks.
Comment 5 Rick L Vinyard Jr 2006-03-05 16:31:50 EST
A few minor tweaks, including cleanup of the Source tag and the %setup section.

Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/bit.spec

SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/4/srpms/bit-0.1.27-2.src.rpm
Comment 6 Michael Schwendt 2006-03-27 18:25:29 EST
* Directory %{_datadir}/bit/ not included in -devel package:

    $ rpmqfcheck.pl bit-devel-0.1.27-2.i386.rpm 
    Orphaned dir: /usr/share/bit

* Package -devel should "Requires: boost-devel" regardless of whether
conexus-devel requires this, too. It includes boost headers itself.

* The two %doc files AUTHORS and COPYING are included in both the -devel
package and the main package, which should be avoided.

* The -devel should "Requires: pkgconfig" since without using
pkg-config's --cflags and --libs commands it would be difficult
to find out all the include search paths and libraries

* DNS-based random mirror download location for SF.net actually is of
the generic form (and command-line compatible!):
http://download.sourceforge.net/conexus/%{name}-%{version}.tar.bz2

* bit-0.1.27/bit/tuplebase.h fails to compile due to extra qualification

* tests/test_istream fails (attachment following...)
Summary:  11 passed, 2 failed
WARNING: 2 TESTS FAILED!!!
Comment 7 Michael Schwendt 2006-03-27 18:26:59 EST
Created attachment 126863 [details]
failed test output
Comment 8 Rick L Vinyard Jr 2006-05-17 00:39:35 EDT
Created attachment 129297 [details]
bit unit test results
Comment 9 Rick L Vinyard Jr 2006-05-17 00:41:11 EDT
Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/bit.spec

SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/5/srpms/bit-0.2.0-1.src.rpm

Many changes and new releases since the last update.

- Orphaned directory
  - Fixed in 0.2.0, data_DIST was missing in Makefile.am that had the .dtd and
.xml files that get installed

- Missing Requires:
  - Added pkgconfig and boost-devel
  - Added cppunit
  - Removed gtkmm24-devel (gtkmm widgets are now in bitgtkmm)

- Duplicate AUTHORS and COPYING are fixed

- Download location changed to
http://prdownloads.sourceforge.net/libbit/%(name)-%(version).tar.bz2
  - I kept getting timeouts with download.sourceforge.net, but prdownloads works.

- Extra qualification is fixed in 0.2.0 release.

- Unit tests migrated to cppunit and extended in 0.2.0. (they're good now; see
attachment)

- Reference doc handling has been changed to reflect the way it was handled in
cairomm
Comment 10 Rick L Vinyard Jr 2006-06-23 00:48:45 EDT
New files:
Spec Name or Url: http://miskatonic.cs.nmsu.edu/pub/bit.spec

SRPM Name or Url:
http://miskatonic.cs.nmsu.edu/pub/fedora/5/srpms/bit-0.2.1-2.src.rpm

Changes:
- Added AUTHORS and COPYING to bit main package
- Changed download dir from prdownloads.sf.net to download.sf.net
- Added cppunit to BuildRequires
- Removed doxygen and graphviz BuildRequires since docs are now in dist
- Changed doc directory to represent new location in dist
Comment 11 Michael Schwendt 2006-07-06 06:56:06 EDT
It must be "BuildRequires: cppunit-devel" instead of cppunit.

Apart from that:

  * reported issues fixed
  * packager is upstream
  * APPROVED

Warnings:

 - compiler warnings about type-punning are worrisome
   (I see dangerous C-style casts in the code)

Suggested cleanup:

 - Remove *.md5, *.map and *.dot files from the HTML documentation
   (91 files in total).

Hints:

 - Note that "mv docs/reference/html ." in your %install section
   breaks --short-circuit installs, which are _very_ useful during
   debugging of spec files. As a rule of thumb, never modify your
   source tree in a way that repeating commands would fail. In this
   case, better "cp" the html tree to somewhere.
   "rpm --short-circuit -ivh bit.spec" should work again and again.

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