Bug 520621 - Review Request: laptop-mode-tools - Scripts to spin down hard drive and save power
Summary: Review Request: laptop-mode-tools - Scripts to spin down hard drive and save ...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2009-09-01 13:27 UTC by Ritesh Raj Sarraf
Modified: 2010-11-03 15:50 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-11-03 15:50:19 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
spec file (3.41 KB, text/plain)
2009-09-18 18:24 UTC, Ritesh Raj Sarraf
no flags Details

Description Ritesh Raj Sarraf 2009-09-01 13:27:19 UTC
Spec URL: http://bazaar.launchpad.net/~laptop-mode-tools-dev/laptop-mode-tools/main/download/head:/laptopmodetools.spec-20090724093155-g4ir4u16u9tyn7ik-1/laptop-mode-tools.spec

SRPM URL: http://www.researchut.com/tmp/laptop-mode-tools-1.50-1.src.rpm

Description:
 Laptop mode is a Linux kernel feature that allows your laptop to save
 considerable power, by allowing the hard drive to spin down for longer
 periods of time. This package contains the userland scripts that are
 needed to enable laptop mode. It includes support for automatically
 enabling laptop mode when the computer is working on batteries. It also
 supports various other power management features, such as starting and
 stopping daemons depending on power mode, automatically hibernating if
 battery levels are too low, and adjusting terminal blanking and X11
 screen blanking.

Comment 1 Till Maas 2009-09-16 20:15:54 UTC
The spec needs a lot of fixing, therefore I added NotReady to the status whiteboard. Please remove it after you have addressed these issues:

1) The spec does not match the srpm, the spec is for version 1.51, but the srpm is for version 1.50
2) GPL is not a valid license tag, it might be GPL+, GPLv2, GPLv2+, ...
You can find more information about this here:
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#GPL_and_LGPL
3) Vendor and Packager should not be used:
https://fedoraproject.org/wiki/Packaging/Guidelines#Tags
4) The Distribution tag should probably not be used, but I asked on fedora-packaging about this
5) The init script should not be started automatically in %post imho, because the user might first want to tweak laptop-mode before it is started
6) %{_usr}/lib/pm-utils/sleep.d must not be owned by laptop-mode-tools, it is owned by filesystem for Fedora Rawhide (F12)
7) The manpages in %files should hava an asterisk appended (*), because in the Fedora buildsystem, the manpages will be gzipped, so the pattern won't match. Also it is not needed to mark them as %doc, this is already done automatically
8) %{_usr}/sbin should be %{_sbindir} and %{_usr}/share %{_datadir}, also it is uncommon to use %{_usr} but to use %{_prefix} instead

You also need to block FE-NEEDSPONSOR, because you do not have submitted any package to Fedora. Here is more information about the whole process:
https://fedoraproject.org/wiki/Package_Review_Process

Comment 2 Ritesh Raj Sarraf 2009-09-18 18:22:18 UTC
(In reply to comment #1)
> The spec needs a lot of fixing, therefore I added NotReady to the status
> whiteboard. Please remove it after you have addressed these issues:
> 
> 1) The spec does not match the srpm, the spec is for version 1.51, but the srpm
> is for version 1.50

Hmmm!! I'm not sure. I think I linked the r1.50 file.

> 2) GPL is not a valid license tag, it might be GPL+, GPLv2, GPLv2+, ...
> You can find more information about this here:
> https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#GPL_and_LGPL

Done

> 3) Vendor and Packager should not be used:
> https://fedoraproject.org/wiki/Packaging/Guidelines#Tags

I used it because the .spec file you reviewed is something I'm going to be shipping with the upstream tarball. One of my goals with that spec is to make it RPM distro generic.

> 4) The Distribution tag should probably not be used, but I asked on
> fedora-packaging about this
Same as 3

> 5) The init script should not be started automatically in %post imho, because
> the user might first want to tweak laptop-mode before it is started
It shouldn't harm. The default settings of laptop-mode-tools thrive to give you a basic power saving profile.
If it is a policy, I can disable it.

> 6) %{_usr}/lib/pm-utils/sleep.d must not be owned by laptop-mode-tools, it is
> owned by filesystem for Fedora Rawhide (F12)

Hmmm!!! I'm not sure how to handle this. We ship a hook there.

> 7) The manpages in %files should hava an asterisk appended (*), because in the
> Fedora buildsystem, the manpages will be gzipped, so the pattern won't match.

Done

> Also it is not needed to mark them as %doc, this is already done automatically
What should be put as the default.

> 8) %{_usr}/sbin should be %{_sbindir} and %{_usr}/share %{_datadir}, also it is
> uncommon to use %{_usr} but to use %{_prefix} instead
> 
Done.  What about /usr/lib ?

> You also need to block FE-NEEDSPONSOR, because you do not have submitted any
> package to Fedora. Here is more information about the whole process:
> https://fedoraproject.org/wiki/Package_Review_Process  
Going through it.

Thank you for reviewing.

I am attaching a separate .spec because the spec I ship with the tarball needs to be generic.

Comment 3 Ritesh Raj Sarraf 2009-09-18 18:24:29 UTC
Created attachment 361704 [details]
spec file

updated spec file for laptop-mode-tools

Comment 4 Ritesh Raj Sarraf 2009-10-09 07:35:45 UTC
laptop-mode-tools version 1.52 has been released.

Any update on this bugzilla ?

Comment 5 Ritesh Raj Sarraf 2010-02-03 07:54:12 UTC
Removing the "NotReady" keyword.

Comment 6 Fabian Affolter 2010-03-02 13:55:50 UTC
There is a release available.

1.53 -- Sat Jan 2 23:42:50 IST 2010

    * Add global enable/disable switch for laptop-mode-tools
    * Add scheduler power saving module for SMT processors. Thanks to John Reilly.
    * Add a new "Auto Modules" mode which enables all modules whitelisted as
      auto with a single configuration setting, ENABLE_AUTO_MODULES.
    * Add LM/NOLM option for Intel SATA Power Management
    * Do a check before trying to write to the SuperHE Control File 

BTW, please provide the Source RPM of your package.  This way it's much easier for reviewers.

Comment 7 Fabian Affolter 2010-03-02 14:05:45 UTC
Some comments on your spec file

- Remove the comments at the beginning of the file.
- I think it would be better to switch to %{_mandir}/man*/*.* for the man pages
  Man pages  should not have %doc and '%docdir %{_mandir}' should be removed
- 'disttag' is missing in 'Release'
- The %install section is missing

Is upstream aware of the bug in the installer?

Comment 8 Ritesh Raj Sarraf 2010-03-02 16:58:38 UTC
Hi, I don't intend to maintain this package in Fedora. This request is more for inclusion of laptop-mode-tools into the Fedora project.

The spec file I added here, I wrote it keeping in mind all RPM distributions. I ship the spec in the source archive and expect users to build using `rpmbuild -ta`, hopefully working on all RPM distributions.

I would appreciate if any of the Fedora Maintainers would work on this.

Comment 9 Jason Tibbitts 2010-11-03 15:50:19 UTC
Since there's no submitter for this package, I'm closing this ticket.

You are welcome to add the package to the wishlist at http://fedoraproject.org/wiki/PackageMaintainers/WishList.  Opening a package review ticket when you don't intend to actually maintain the package is simply not helpful.


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