Bug 226411 - Merge Review: setserial
Summary: Merge Review: setserial
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
medium
medium
Target Milestone: ---
Assignee: Tim Waugh
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
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:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-02-07 18:34:10 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wolfy: fedora-review+


Attachments (Terms of Use)
fixes mandir and allows creating a debuginfo rpm (653 bytes, text/plain)
2007-02-06 20:33 UTC, manuel wolfshant
no flags Details
replaces Red Hat with Fedora in instructions (554 bytes, text/plain)
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 (4.04 KB, text/plain)
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.