Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226411 - Merge Review: setserial
Summary: Merge Review: setserial
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tim Waugh
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 20:58 UTC by Nobody's working on this, feel free to take it
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-02-07 18:34:10 UTC
wolfy: fedora-review+


Attachments (Terms of Use)
fixes mandir and allows creating a debuginfo rpm (deleted)
2007-02-06 20:33 UTC, manuel wolfshant
no flags Details
replaces Red Hat with Fedora in instructions (deleted)
2007-02-06 20:34 UTC, manuel wolfshant
no flags Details
fixed spec. Beware, release is the SAME as the one that was in CVS/devel at 20:15 GMT/Feb 6th 2007 (deleted)
2007-02-06 20:37 UTC, manuel wolfshant
no flags Details

Description Nobody's working on this, feel free to take it 2007-01-31 20:58:17 UTC
Fedora Merge Review: setserial

http://cvs.fedora.redhat.com/viewcvs/devel/setserial/
Initial Owner: twaugh@redhat.com

Comment 1 manuel wolfshant 2007-02-06 20:32:58 UTC
Review for Release: 19.2.2

MUSTFIX:
there are a couple of problems with the spec and the patches
* the fhs patch has two errors
+mandir = @bindir@ <-- this should be @mandir@
+       $(STRIP) $(DESTDIR)$(bindir)/setserial -< should not be at all, leads to
empty debuginfo
* the readme patch should include references to Fedora, not Red Hat
* the spec does not include the preferred BUILDROOT, does not honor SMP
flags,uses %makeinstall instead of make install
Warning from rpmlint: Summary ends with dot 


I will attach the fixes for all of the above. Please use whatever you find
useful and once corrected I will do the formal full review.


Comment 2 manuel wolfshant 2007-02-06 20:33:58 UTC
Created attachment 147516 [details]
fixes mandir and allows creating a debuginfo rpm

Comment 3 manuel wolfshant 2007-02-06 20:34:43 UTC
Created attachment 147517 [details]
replaces Red Hat with Fedora in instructions

Comment 4 manuel wolfshant 2007-02-06 20:37:00 UTC
Created attachment 147518 [details]
fixed spec. Beware, release is the SAME as the one that was in CVS/devel at 20:15 GMT/Feb 6th 2007

Comment 5 manuel wolfshant 2007-02-07 00:21:40 UTC
Forgot to mention, I did not add %{dist} to the Release field. If possible, it
should be added.

Comment 6 Tim Waugh 2007-02-07 10:42:40 UTC
Thanks.  Tagged and built as 2.17-20.fc7.

Comment 7 manuel wolfshant 2007-02-07 11:32:45 UTC
The public accessible cvs server is not sync-ed yet. I'll do the review tonight.

Comment 8 manuel wolfshant 2007-02-07 17:48:04 UTC
Formal review for release 2.17-20.fc7:

- package meets naming guidelines
- package meets packaging guidelines
- license is GPL (hence OK), matches source; upstream does not include the
license, so it isn't included in the package either
- spec file legible, in am. english
- source matches upstream, last available version, sha1sum
68824494a0b5700f7e999564a59358bf34f79eb1  setserial-2.17.tar.gz
- package bilds in mock for devel/x86_64
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all files and directories that it creates, does take not take ownership
of foreign files/directories
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no static, .pc, .la files
- no need for .desktop file
- rpmlint is silent on the source; there is one warning for the generated binary:
W: setserial spurious-executable-perm /usr/share/doc/setserial-2.17/rc.serial
It can be ignored, this is meant as an initscript which ( if needed ) must be
installed in /etc/init.d anyway

SHOULD
- builds cleanly in mock
- runs as advertised

TODO
- upstream should be bugged to included the license in the supplied tar.gz


APPROVED

Comment 9 Till Maas 2007-09-01 13:34:26 UTC
depending on FE-ACCEPT is wrong, blocking it is not needed when fedora-review+
is set.


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