Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 183953
Summary: | Review Request: bit (A bit-oriented data stream parser and gtkmm widget set) | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Rick L Vinyard Jr <rvinyard> | ||||||
Component: | Package Review | Assignee: | Michael Schwendt <bugs.michael> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | ||||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2006-07-07 15:58:16 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 163779, 192052 | ||||||||
Attachments: |
|
Description
Rick L Vinyard Jr
2006-03-04 01:32:38 UTC
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). 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 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 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. 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 * 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!!! Created attachment 126863 [details]
failed test output
Created attachment 129297 [details]
bit unit test results
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 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 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. |