> spindown.spec: W: invalid-url Source0:
> http://spindown.googlecode.com/files/spindown-0.4.0.tar.gz HTTP Error 404: 
> Not Found

Just note this is a valid url, but google blocks certain clients probably.

Also the spelling warnings should be okay I suppose.

I'll review this (and your other submission), and assuming all goes well sponsor you eventually, removing FE_NEEDSPONSOR blocker.

Full review done:

Good:
- rpmlint checks return:
spindown.src: W: name-repeated-in-summary C spindown
spindown.src: W: spelling-error %description -l en_US sg -> chg, gs, sf
spindown.src: W: spelling-error %description -l en_US swappable -> trappable
spindown.src: W: spelling-error %description -l en_US hda -> had, ha, Ida
spindown.src: W: spelling-error %description -l en_US sdb -> db, sob, sub
spindown.src: W: invalid-url Source0: http://spindown.googlecode.com/files/spindown-0.4.0.tar.gz HTTP Error 404: Not Found
spindown.x86_64: W: name-repeated-in-summary C spindown
spindown.x86_64: W: spelling-error %description -l en_US sg -> chg, gs, sf
spindown.x86_64: W: spelling-error %description -l en_US swappable -> trappable
spindown.x86_64: W: spelling-error %description -l en_US hda -> had, ha, Ida
spindown.x86_64: W: spelling-error %description -l en_US sdb -> db, sob, sub
spindown.x86_64: W: no-documentation
spindown.x86_64: W: no-manual-page-for-binary spindownd
 These can all be ignored
- package meets naming guidelines
- package meets packaging guidelines
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- no need for .desktop file

Needs work:
- rpmlint checks return:
spindown-debuginfo.x86_64: E: debuginfo-without-sources
 That is not good, this is likely caused by RPM_OPT_FLAGS not being used, see
 below.
 
-license should be GPLv3+

-the requires for the post / preun scripts are wrong, and the output
 of the service command should be silenced, please use the template requires
 + scripts from here:
 http://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_in_spec_file_scriptlets

-RPM_OPT_FLAGS are not used during compilation. You call make with:
 OPT="$RPM_OPT_FLAGS" but that does not seem to do the trick, the output during
 building shows:
 g++ -O1 -c src/main.cpp

-You're not using %{?_smp_mflags} when calling make from %build, if you've tried
 so and it does not work, please add a comment stating so

-src/ininiparser3.0b contains an embedded copy if the iniparser library,
 luckily the exact same version is already packaged for Fedora, so fixing this
 is easy, just rm -rf src/ininiparser3.0b in %prep, fixes the includes to use
 the headers from the iniparser-devel package and add -liniparser to the
 linking command in the Makefile

-the initscript:
 - tries to start shutdownd when no /etc/shutdown.conf is present
 - is missing the usual printing of [ OK] / [FAILED]
 - you may want to consider writing a Fedora specific initscript, using
   the template from:
   http://fedoraproject.org/wiki/Packaging/SysVInitScript
   That should be easy to adapt to shutdownd

- You're not packaing any docs, you MUST package the COPYING file as %doc,
  you should also package the CHANGELOG README TODO and spindown.conf.example
  files as %doc

I belive all issues are fixed now in spindown-0.4.0-2:

Spec URL: http://www.physics.muni.cz/~cermak/spindown-0.4.0-2/spindown.spec
SRPM URL: http://www.physics.muni.cz/~cermak/spindown-0.4.0-2/spindown-0.4.0-2.fc14.src.rpm
Patch0 URL: http://www.physics.muni.cz/~cermak/spindown-0.4.0-2/spindown-0.4.0-initscript.patch
Patch1 URL: http://www.physics.muni.cz/~cermak/spindown-0.4.0-2/spindown-0.4.0-Makefile.patch
Patch2 URL: http://www.physics.muni.cz/~cermak/spindown-0.4.0-2/spindown-0.4.0-iniparser.patch

All changes are described in the changelog. Because the project is not under version control yet, I'm creating patches against pure sources. For this reason patches changed (for example [1] differs from [2]). Also I'm omitting the licence change announcement on fedora-devel-list in this case. Here are rpmlint results:

+ rpmlint spindown.spec
spindown.spec: W: invalid-url Source0: http://spindown.googlecode.com/files/spindown-0.4.0.tar.gz HTTP Error 404: Not Found
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

+ rpmlint spindown-0.4.0-2.fc14.src.rpm
spindown.src: W: spelling-error %description -l en_US sg -> SG, chg, cig
spindown.src: W: spelling-error %description -l en_US hdparm -> hardpan, harmed, Parma
spindown.src: W: spelling-error %description -l en_US swappable -> trappable, mappable, unflappable
spindown.src: W: spelling-error %description -l en_US hda -> had, hd, ha
spindown.src: W: spelling-error %description -l en_US sdb -> sd, sb, db
spindown.src: W: invalid-url Source0: http://spindown.googlecode.com/files/spindown-0.4.0.tar.gz HTTP Error 404: Not Found
1 packages and 0 specfiles checked; 0 errors, 6 warnings.

+ rpmlint spindown-debuginfo-0.4.0-2.fc14.x86_64.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

+ rpmlint spindown-0.4.0-2.fc14.x86_64.rpm
spindown.x86_64: W: spelling-error %description -l en_US sg -> SG, chg, cig
spindown.x86_64: W: spelling-error %description -l en_US hdparm -> hardpan, harmed, Parma
spindown.x86_64: W: spelling-error %description -l en_US swappable -> trappable, mappable, unflappable
spindown.x86_64: W: spelling-error %description -l en_US hda -> had, hd, ha
spindown.x86_64: W: spelling-error %description -l en_US sdb -> sd, sb, db
spindown.x86_64: W: no-manual-page-for-binary spindownd
1 packages and 0 specfiles checked; 0 errors, 6 warnings.

The package does not explicitly require sg3_utils because custom power management binaries (like sg_start or hdparm) can be used. 

I also performed sanity testing of the package on a machine with multiple disks.

-------------------
[1] http://www.physics.muni.cz/~cermak/spindown-0.4.0-2/spindown-0.4.0-Makefile.patch
[2] http://www.physics.muni.cz/~cermak/spindown/spindown-0.4.0-Makefile.patch

Much better now, almost there.

Needs work:
- You're installing the example config file as /etc/spindown.conf now,
  no need to include at is as %doc then
- Thanks for doing the new initscript, but I'm afraid it needs some work:
  Must Fix:
  - The LSB HEADER is entirely empty, fill in the Short Description and
    Description and remove all the other lines.
  - statuspipe="/tmp/spindown.status"
    Is vulnerable to symlinks attacks, please make this /var/run/spindown.status
    instead
  - the $prog in "status -p /var/run/spindownd.pid $prog" must be $exec
  - start() is missing a "return $retval" at the end

  Should fix:
  - Given that you've effectively written a new script, please add it as
    Source1: (without an url) and then in $setup do:
    cp -p %{SOURCE1} .
    Rather then making it a hard to read patch. This will also make
    making future changes to it a lot easier.
  - Drop the unused RETVAL variable
  - Please use a pidfile variable (like lockfile and statuspipe) rather then
    writing out /var/run/spindownd.pid in full everywhere
  - This part:
    echo -n $"Starting $prog: "
    if [ $UID -ne 0 ] ; then   
        failure
        echo -e "\nUser has insufficient privilege."
        exit 4
    fi
    if [ ! -x $exec ]; then
        failure
        echo -e "\nDaemon binary not executable."
        exit 5
    fi
    if [ ! -f $config ]; then
        failure
        echo -e "\nConfig file missing."
        exit 6
    fi

    Deviates from how most Fedora init scripts do it, please change this to:

    [ $UID -eq 0 ] || exit 4
    [ -x $exec ] || exit 5
    [ -f $config ] || exit 6
    echo -n $"Starting $prog: "

  - You can likely (and if it works should) write this bit:

    $exec -d -f $statuspipe -c $config -p /var/run/spindownd.pid;
    retval=$?
    [ "$retval" -eq 0 ] && success || failure
    echo

    As:

    daemon $exec -d -f $statuspipe -c $config -p /var/run/spindownd.pid;
    retval=$?
    echo

  - stop() can be simplified to just:
    stop() {
        echo -n $"Stopping $prog: "
        killproc -p /var/run/spindownd.pid $exec
        retval=$?
        echo
        [ $retval -eq 0 ] && rm -f $lockfile
        return $retval
    }

  - judging from the old initscript reload should be:
    reload() {
        killproc -p /var/run/spindownd.pid $exec -HUP
    }

    And force_reload should then call reload rather then restart

Created spindown-0.4.0-3.fc14 addressing the aforementioned issues, see [1]. 
I hope it'll be fine now. Please review.

-------------------
[1] http://www.physics.muni.cz/~cermak/spindown-0.4.0-3/

(In reply to comment #6)
> Created spindown-0.4.0-3.fc14 addressing the aforementioned issues, see [1]. 
> I hope it'll be fine now. Please review.
> 
> -------------------
> [1] http://www.physics.muni.cz/~cermak/spindown-0.4.0-3/

Hi,

Thanks, looks good now: approved.

Regards,

Hans

New Package SCM Request
=======================
Package Name: spindown
Short Description: Daemon that can spin idle disks down
Owners: mcermak
Branches: f14 f15
InitialCC: jwrdegoede

Git done (by process-git-requests).

spindown-0.4.0-3.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/spindown-0.4.0-3.fc15

spindown-0.4.0-3.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/spindown-0.4.0-3.fc14

spindown-0.4.0-3.fc14 has been pushed to the Fedora 14 testing repository.

spindown-0.4.0-3.fc14 has been pushed to the Fedora 14 stable repository.

spindown-0.4.0-3.fc15 has been pushed to the Fedora 15 stable repository.