Bug 884674 - Review Request: kde-plasma-alsa-volume -- ALSA Volume Control plasmoid
Summary: Review Request: kde-plasma-alsa-volume -- ALSA Volume Control plasmoid
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: Trivial
Depends On:
Blocks: kde-reviews
TreeView+ depends on / blocked
 
Reported: 2012-12-06 14:30 UTC by Fl@sh
Modified: 2013-06-05 10:29 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2013-02-24 08:43:15 UTC
Type: ---
Embargoed:
rdieter: fedora-review+


Attachments (Terms of Use)

Description Fl@sh 2012-12-06 14:30:23 UTC
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 20:49:17 UTC
Uhm, you do know that KMix has an ALSA mode, too (KMIX_PULSEAUDIO_DISABLE=1)?

Comment 2 Fl@sh 2012-12-06 21:09:31 UTC
(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 12:49:11 UTC
new successfull build : http://koji.fedoraproject.org/koji/taskinfo?taskID=4772537

Comment 4 Sébastien Boisvert 2013-01-11 15:08:40 UTC
The SRPM gives HTTP 404 Not Found.

Comment 6 Mario Blättermann 2013-01-13 19:42:15 UTC
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 12:15:07 UTC
(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 16:54:05 UTC
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 17:05:06 UTC
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 18:05:53 UTC
OK, augment my suggestion for 2 with Kevin's, and add:
BuildRequires: kdelibs4-devel
as well.

Comment 14 Rex Dieter 2013-02-10 21:51:06 UTC
If using plasma-dataengine-depextractor approach, you need to 
BuildRequires: kdelibs4-devel

Comment 16 Rex Dieter 2013-02-11 16:51:34 UTC
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 22:07:39 UTC
Thanks!

Comment 18 Fl@sh 2013-02-12 07:38:55 UTC
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 13:28:40 UTC
Git done (by process-git-requests).

Comment 20 Fedora Update System 2013-02-14 21:44:38 UTC
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 21:44:53 UTC
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-16 01:02:27 UTC
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 08:43:17 UTC
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 08:55:01 UTC
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 10:05:20 UTC
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 10:29:52 UTC
Already exists.


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