[Bug 822730] Review Request: laptop-mode-tools - Tools for power savings based on battery/AC status

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--- Comment #22 from Jaroslav Škarvada <jskarvad@xxxxxxxxxx> ---
(In reply to Claudio from comment #21)
> I've actully tried to fix a few things that had been mentioned in the older
> posts, let me know.

Comments:

* Please also provide the new SRPM, especially in case you rebased the package.

* Please add %install section [1]. Installing content from %build section by
e.g.:
%{__install} -Dp -m755 etc/init.d/laptop-mode %{buildroot}%{_initrddir}
is unacceptable.

* In Fedora, we do not use the Packager tag, please drop it. Credits are
already in the changelog, and the actual ownership of the package is better
tracked in the pkgdb.

* Please use '%setup -q' [2]

* Macro forms of system executables SHOULD NOT be used... For example, rm
should be used in preference to %{__rm} [3]

* Fedora (as of F-10) does not require the presence of the BuildRoot tag in the
spec and if one is defined it will be ignored [4]. This tag is needed only if
packaging/targeting the package to EPEL-5 (which is IMHO not this case). Also
please do not explicitly clean the build root (i.e rm -rf %{buildroot}).

* Please use systemd macros to install the service file [5].

* Please do not use macros in changelog, prefix the macros by another %, i.e.
use the following in the changelog:
- Restart acpid after %%{__install} -Dping.

* Please use macro %{_datadir} instead of %{_usr}/share.

* Please package initrd stuff to sysvinit subpackage.

* You msut not own the %{_sysconfdir}/acpi/ subdirs (events, actions), there
are already owned by acpid package, you should require the acpid package.

* The same for apm.

* But you should probably own the %{_sysconfdir}/power directory.

* Please use Requires: pkg1, pkg2, ... (or leave the comma) to be consistent in
formatting with other parameters instead of:
requires:udev
requires:acpid
...

* /sbin/chkconfig calls definitely belongs to the sysvinit subpackage [6].


[1]
https://fedoraproject.org/wiki/How_to_create_an_RPM_package?rd=PackageMaintainers/CreatingPackageHowTo#.25install_section

[2]
http://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25prep_section:_.25setup_command

[3] http://fedoraproject.org/wiki/Packaging:Guidelines#Macros

[4] http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

[5] http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

[6]
http://fedoraproject.org/wiki/Packaging:SysVInitScript#Initscripts_in_addition_to_systemd_unit_files

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=3UOpNNMICq&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]