Bug 884674 - Review Request: kde-plasma-alsa-volume -- ALSA Volume Control plasmoid
Review Request: kde-plasma-alsa-volume -- ALSA Volume Control plasmoid
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Extras Quality Assurance
Trivial
:
Depends On:
Blocks: kde-reviews
  Show dependency treegraph
 
Reported: 2012-12-06 09:30 EST by Fl@sh
Modified: 2013-06-05 06:29 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-02-24 03:43:15 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rdieter: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Fl@sh 2012-12-06 09:30:23 EST
Spec URL: https://github.com/F1ash/plasmaVolume/blob/master/kde-plasma-alsa-volume.spec
SRPM URL: http://kojipkgs.fedoraproject.org//work/tasks/3303/4763303/kde-plasma-alsa-volume-0.41.2-1.fc18.src.rpm
Description:
kde-plasma-alsa-volume
ALSA Volume Control plasmoid

Fedora Account System Username: f1ash

Addition:
succesful build http://koji.fedoraproject.org/koji/taskinfo?taskID=4763303
Comment 1 Kevin Kofler 2012-12-06 15:49:17 EST
Uhm, you do know that KMix has an ALSA mode, too (KMIX_PULSEAUDIO_DISABLE=1)?
Comment 2 Fl@sh 2012-12-06 16:09:31 EST
(In reply to comment #1)
> Uhm, you do know that KMix has an ALSA mode, too (KMIX_PULSEAUDIO_DISABLE=1)?

but this plasmoid is more convenient, because it allows each device to reflect on the panel (or workspace) for control. ( http://kde-look.org/content/show.php/ALSA+Volume+Control+plasmoid?content=138990 )
Comment 3 Fl@sh 2012-12-10 07:49:11 EST
new successfull build : http://koji.fedoraproject.org/koji/taskinfo?taskID=4772537
Comment 4 Sébastien Boisvert 2013-01-11 10:08:40 EST
The SRPM gives HTTP 404 Not Found.
Comment 6 Mario Blättermann 2013-01-13 14:42:15 EST
Some initial issues:

Please provide a more informative description, if possible. Repeating the package name doesn't make much sense. You should know what to do because you are the upstream author ;)

The source tarball is not downloadable:

$ wget http://cloud.github.com/downloads/F1ash/plasmaVolume/kde-plasma-alsa-volume-0.41.2.tar.bz2
--2013-01-13 20:37:09--  http://cloud.github.com/downloads/F1ash/plasmaVolume/kde-plasma-alsa-volume-0.41.2.tar.bz2
Auflösen des Hostnamen »cloud.github.com (cloud.github.com)«... 54.240.162.142, 54.240.162.98, 54.240.162.18, ...
Verbindungsaufbau zu cloud.github.com (cloud.github.com)|54.240.162.142|:80... verbunden.
HTTP-Anforderung gesendet, warte auf Antwort... 403 Forbidden
2013-01-13 20:37:10 FEHLER 403: Forbidden.
Comment 7 Fl@sh 2013-01-15 07:15:07 EST
(In reply to comment #6)
> Some initial issues:
> 
> Please provide a more informative description, if possible. Repeating the
> package name doesn't make much sense. You should know what to do because you
> are the upstream author ;)
> 
> The source tarball is not downloadable:
> ...
Fixed both:

https://raw.github.com/F1ash/plasmaVolume/master/kde-plasma-alsa-volume.spec
Comment 10 Rex Dieter 2013-02-09 11:54:05 EST
naming: ok

sources: NOT ok.  
md5sum checksum in SRPM:
5c5167a012f43ccd0204fe600ef32095  kde-plasma-alsa-volume-0.41.2.tar.gz
md5sum of Source0 URL content:
146e0b1dae8ef5cfaa47b5fd75b93bdb  kde-plasma-alsa-volume-0.41.2.tar.gz

1.  srpm MUST contain verifiable sources

license: ok

2.  MUST: add missing runtime dependency for plasma-scriptengine-python. i'd suggest simply adding this to the top of your %build section:
if [ -x %{_bindir}/plasma-dataengine-depextractor ] ; then
  plasma-dataengine-depextractor .
fi
then, it'll get added automatically for you.  Doing this, you could probably even drop the explicit,
Requires: PyKDE4

macros: ok

scriplets: n/a, ok

3.  SHOULD simplify .spec and remove deprecated/used "Group:" tag


Otherwise, relatively small, simple package.  Address at least MUST items 1,2 and i'll approve this.
Comment 11 Kevin Kofler 2013-02-09 12:05:06 EST
Dependencies on script engines are actually detected automatically (the required metadata for those is already in the upstream metadata.desktop), but it needs a BuildRequires of (at least) kde-settings, which is missing in your package.

If you're going to run the plasma-dataengine-depextractor (which you should, really, because you don't know what data engines upstream will start using in the future, though currently I can't see any being used in the code), you need BuildRequires: kdelibs4-devel, which will also drag in kde-settings.
Comment 12 Rex Dieter 2013-02-09 13:05:53 EST
OK, augment my suggestion for 2 with Kevin's, and add:
BuildRequires: kdelibs4-devel
as well.
Comment 14 Rex Dieter 2013-02-10 16:51:06 EST
If using plasma-dataengine-depextractor approach, you need to 
BuildRequires: kdelibs4-devel
Comment 16 Rex Dieter 2013-02-11 11:51:34 EST
fyi, for future reference, it is best practice to bump
Release:
and add new %changelog entries when making changes (esp those in a package review), so it's easier to track what has changed.  thanks.

sources: ok now
b8709502c561d8fc5ab6aaccccda3e02  0.41.2.tar.gz

Looks good, APPROVED.
Comment 17 Fl@sh 2013-02-11 17:07:39 EST
Thanks!
Comment 18 Fl@sh 2013-02-12 02:38:55 EST
New Package SCM Request
=======================
Package Name: kde-plasma-alsa-volume
Short Description:  ALSA Volume Control plasmoid
Owners: f1ash
Branches: f17 f18
InitialCC:
Comment 19 Gwyn Ciesla 2013-02-12 08:28:40 EST
Git done (by process-git-requests).
Comment 20 Fedora Update System 2013-02-14 16:44:38 EST
kde-plasma-alsa-volume-0.41.2-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/kde-plasma-alsa-volume-0.41.2-1.fc18
Comment 21 Fedora Update System 2013-02-14 16:44:53 EST
kde-plasma-alsa-volume-0.41.2-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/kde-plasma-alsa-volume-0.41.2-1.fc17
Comment 22 Fedora Update System 2013-02-15 20:02:27 EST
kde-plasma-alsa-volume-0.41.2-1.fc17 has been pushed to the Fedora 17 testing repository.
Comment 23 Fedora Update System 2013-02-24 03:43:17 EST
kde-plasma-alsa-volume-0.41.2-1.fc18 has been pushed to the Fedora 18 stable repository.
Comment 24 Fedora Update System 2013-02-24 03:55:01 EST
kde-plasma-alsa-volume-0.41.2-1.fc17 has been pushed to the Fedora 17 stable repository.
Comment 25 Fl@sh 2013-06-05 06:05:20 EDT
New Package SCM Request
=======================
Package Name: kde-plasma-alsa-volume
Short Description:  ALSA Volume Control plasmoid
Owners: f1ash
Branches: f19
InitialCC:
Comment 26 Gwyn Ciesla 2013-06-05 06:29:52 EDT
Already exists.

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