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 226104 - Merge Review: logrotate
Summary: Merge Review: logrotate
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ruben Kerkhof
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 19:34 UTC by Nobody's working on this, feel free to take it
Modified: 2007-11-30 22:11 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-06-20 07:56:22 UTC
ruben: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 19:34:54 UTC
Fedora Merge Review: logrotate

http://cvs.fedora.redhat.com/viewcvs/devel/logrotate/
Initial Owner: pvrabec@redhat.com

Comment 1 Ruben Kerkhof 2007-02-03 22:28:10 UTC
* RPM name is OK
* Builds fine in mock
* File list looks OK

Needs work:
* BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
  (wiki: PackagingGuidelines#BuildRoot)
* Missing SMP flags. If it doesn't build with it, please add a comment
  (wiki: PackagingGuidelines#parallelmake)
* Spec file: some paths are not replaced with RPM macros
  (wiki: QAChecklist item 7)
* No downloadable source. Please give the full URL in the Source tag.
* The package should contain the text of the license
  (wiki: Packaging/ReviewGuidelines)

Notes:
* There's no need to add -g to RPM_OPT_FLAGS, it's included by default
* Use DESTDIR instead of PREFIX in %install
* Preserve timestamps with install -p in %install
* Is it necessary to create the empty logrotate.status?

Rpmlint output:
rpmlint of logrotate-3.7.4-11.fc6.i386.rpm:W: logrotate no-url-tag
E: logrotate zero-length /var/lib/logrotate.status


Comment 2 Peter Vrabec 2007-02-08 16:12:58 UTC
>No downloadable source. Please give the full URL in the Source tag.
There isn't any URL you can download sources from. RH is upstream for 
logrotate.

> Use DESTDIR instead of PREFIX in %install
I can't use DESTDIR, since there isn't standart Makefile in logrotate.

> Is it necessary to create the empty logrotate.status?
bz #55809

see: logrotate-3.7.4-12.fc7

Comment 3 Ruben Kerkhof 2007-02-08 21:25:48 UTC
>> No downloadable source. Please give the full URL in the Source tag.
> There isn't any URL you can download sources from. RH is upstream for logrotate.

Agreed.

>> Use DESTDIR instead of PREFIX in %install
> I can't use DESTDIR, since there isn't standart Makefile in logrotate.

Ok.

>> Is it necessary to create the empty logrotate.status?
> bz #55809

Ok, I'm not sure this is a bug, but is doesn't do any harm either.

One thing left: you haven't converted all paths to macro's
There's /etc in %install, replace that with %{_sysconfdir}
Replace /var with %{_localstatedir}

Thanks,

Ruben

Comment 4 Peter Vrabec 2007-02-09 10:25:38 UTC
fixed in logrotate-3.7.4-13.fc7

Comment 5 Ruben Kerkhof 2007-02-10 11:41:48 UTC
Thanks.

I don't see any blockers, so this package is approved.


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