Bug 226411 - Merge Review: setserial
Merge Review: setserial
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tim Waugh
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 15:58 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-02-07 13:34:10 EST
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 15:33 EST, manuel wolfshant
no flags Details
replaces Red Hat with Fedora in instructions (554 bytes, text/plain)
2007-02-06 15:34 EST, 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 15:37 EST, manuel wolfshant
no flags Details

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 15:58:17 EST
Fedora Merge Review: setserial

http://cvs.fedora.redhat.com/viewcvs/devel/setserial/
Initial Owner: twaugh@redhat.com
Comment 1 manuel wolfshant 2007-02-06 15:32:58 EST
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 15:33:58 EST
Created attachment 147516 [details]
fixes mandir and allows creating a debuginfo rpm
Comment 3 manuel wolfshant 2007-02-06 15:34:43 EST
Created attachment 147517 [details]
replaces Red Hat with Fedora in instructions
Comment 4 manuel wolfshant 2007-02-06 15:37:00 EST
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-06 19:21:40 EST
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 05:42:40 EST
Thanks.  Tagged and built as 2.17-20.fc7.
Comment 7 manuel wolfshant 2007-02-07 06:32:45 EST
The public accessible cvs server is not sync-ed yet. I'll do the review tonight.
Comment 8 manuel wolfshant 2007-02-07 12:48:04 EST
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 09:34:26 EDT
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.