[Bug 225237] Merge Review: acpid

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: acpid


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225237


kevin at tummy.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|kevin at tummy.com             |pknirsch at redhat.com
               Flag|fedora-review?              |fedora-review-




------- Additional Comments From kevin at tummy.com  2007-02-03 12:49 EST -------

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
3aff94e92186e99ed5fd6dcee2db7c74  acpid-1.0.4.tar.gz
3aff94e92186e99ed5fd6dcee2db7c74  acpid-1.0.4.tar.gz.1
OK - Package needs ExcludeArch
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
See below - Package has correct buildroot
OK - Package is code or permissible content.

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane:

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
OK - Should have sane scriptlets.
See below - Should have dist tag
OK - Should package latest version
10 bugs - check for outstanding bugs on package.

Issues:

1. Buildroot should be changed to standard. Should add smp_mflags?

2. Might include COPYING, README, Changelog, TODO as doc files?

3. rpmlint our pal says:

rpmlint on ./acpid-1.0.4-5.i386.rpm
W: acpid conffile-without-noreplace-flag /etc/acpi/events/power.conf
W: acpid conffile-without-noreplace-flag /etc/acpi/events/video.conf
W: acpid conffile-without-noreplace-flag /etc/logrotate.d/acpid

Should all be noreplace?

E: acpid non-readable /usr/sbin/acpid 0750
E: acpid non-standard-executable-perm /usr/sbin/acpid 0750

Should this really be non readable by anyone? Why?
If so, perhaps a rpmlint bug should be filed?

W: acpid service-default-enabled /etc/rc.d/init.d/acpid
Should this really be enabled on all machines?
Are there cases where it might not be desired by default?

rpmlint on ./acpid-1.0.4-5.src.rpm
W: acpid strange-permission acpid.init 0755
W: acpid prereq-use /sbin/chkconfig, /sbin/service

Should perhaps be:

Requires(post): /sbin/chkconfig
Requires(preun): /sbin/chkconfig
Requires(preun): /sbin/service

See:
http://www.fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-a6d7a1ed9d77dbb8d4af067378a79b838aebb20a

W: acpid setup-not-quiet

Should add -q to setup.

4. In the files section:

%verify(not md5 size mtime) %ghost %config(missingok,noreplace) /var/log/acpid

Why all this?

/usr/bin/acpi_listen
/usr/sbin/acpid

Should those have %{_bindir} and %{_sbindir} ?

/usr/share/man/man8/acpid.8.gz
/usr/share/man/man8/acpi_listen.8.gz

Should have %{_mandir} ?

5. You might look at the outstanding bugs on this package.
In particular the bugs asking for better scripts might stand to have a
response like "please submit your outstanding scripts for inclusion"


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.