Bug 195223 - Review Request: pavucontrol: Volume control for PulseAudio
Review Request: pavucontrol: Volume control for PulseAudio
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Package Reviews List
:
Depends On: 195221
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-06-14 11:24 EDT by Pierre Ossman
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-09 14:05:22 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Pierre Ossman 2006-06-14 11:24:16 EDT
Spec URL: http://homes.drzeus.cx/~drzeus/polypaudio/pavucontrol.spec
SRPM URL: http://homes.drzeus.cx/~drzeus/polypaudio/pavucontrol-0.9.1-1.src.rpm
Description:
Polypaudio Volume Control (pavucontrol) is a simple GTK based volume control
tool ("mixer") for the Polypaudio sound server. In contrast to classic mixer
tools this one allows you to control both the volume of hardware devices and
of each playback stream separately.
Comment 3 Brian Pepple 2006-09-06 19:42:12 EDT
Quick note:

1. Desktop file handling does not conform to Fe guidelines.  Refer to
http://fedoraproject.org/wiki/Packaging/Guidelines#head-254ddf07aae20a23ced8cecc219d8f73926e9755
Comment 4 Pierre Ossman 2006-09-08 01:10:49 EDT
I read that portion of the guidelines as relating to the case of when you have
to include a .desktop file yourself (i.e. when upstream doesn't have one). If
not, I would have expected a "rm" somewhere in the script to first kill off the
.desktop file the Makefiles have installed.
Comment 5 Brian Pepple 2006-09-08 08:47:02 EDT
You would simply add something like:

desktop-file-install --vendor fedora --delete-original	\
  --dir $RPM_BUILD_ROOT%{_datadir}/applications   	\
  --add-category X-Fedora			        \
  $RPM_BUILD_ROOT%{_datadir}/applications/%{name}.desktop

to the install section of your spec file.
Comment 6 Rex Dieter 2006-09-08 09:09:17 EDT
So that the .desktop file (name) doesn't vary from upstream, I'd suggest 
using:
--vendor=""
instead.
Comment 7 Rex Dieter 2006-09-08 09:14:01 EDT
spec looks clean,simple, just
* MUST: add to %install section
desktop-file-install \
  --dir $RPM_BUILD_ROOT%{_datadir}/applications         \
  --add-category="X-Fedora" --vendor=""                  \
  $RPM_BUILD_ROOT%{_datadir}/applications/%{name}.desktop

do that, and pending my confirmation for building in mock and rpmlint sanity 
checking, I'll APPROVE this.
Comment 9 Brian Pepple 2006-09-08 18:33:09 EDT
(In reply to comment #6)
> So that the .desktop file (name) doesn't vary from upstream, I'd suggest 
> using:
> --vendor=""
> instead.

Rex, maybe I'm missing something here.  Why does it matter if you are setting
the vendor?

Comment 10 Rex Dieter 2006-09-08 18:58:18 EDT
> Why does it matter if you are setting the vendor?

What *matters* is that .desktop files not get renamed, and adding --vendor does
just that.
Comment 11 Brian Pepple 2006-09-08 19:18:17 EDT
(In reply to comment #10)
> > Why does it matter if you are setting the vendor?
> 
> What *matters* is that .desktop files not get renamed, and adding --vendor does
> just that.

I was aware that adding the vendor changes the desktop filename, but I guess my
question is why does it matter if the desktop file gets renamed?  Plenty of
packages in extras do it.

Comment 12 Rex Dieter 2006-09-08 20:23:30 EDT
> why does it matter if the desktop file gets renamed? 

Lots of reasons, one of which is menu editing.

> Plenty of packages in extras do it.

I know, but they (mostly) shouldn't have.  But, since the files were renamed
once, they probably ought to stay that way (else we'd comitt the sin of renaming
them *again*).
Comment 14 Rex Dieter 2006-09-09 08:34:55 EDT
One possible simplification is replace in %files:
%dir %{_datadir}/pavucontrol
%{_datadir}/pavucontrol/pavucontrol.glade
with
%{_datadir}/pavucontrol/

Comment 15 Rex Dieter 2006-09-09 08:36:08 EDT
mock builds fine, rpmlint is happy:
$rpmlint pavucontrol-0.9.4-2.fc6.i386.rpm 386.rpm 
W: pavucontrol incoherent-version-in-changelog 0.9.4-2 0.9.4-2.fc6

APPROVED.
Comment 16 Lennart Poettering 2007-07-30 13:34:38 EDT
Package Change Request
======================
Package Name: pavucontrol
Updated Fedora Owners: lpoetter@redhat.com,drzeus-bugzilla@drzeus.cx
Comment 17 Lennart Poettering 2007-07-30 13:37:26 EDT
Sorry, I forgot the justification for the owner change: I am upstream for this
package and Pierre agreed to co-maintain this package with me from now on.
Comment 18 Jason Tibbitts 2007-07-30 13:37:51 EDT
CVS done.

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