Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 224402 - Review Request: gstreamer-plugins-pulse - GStreamer 0.10 plugin for the PulseAudio sound server
Summary: Review Request: gstreamer-plugins-pulse - GStreamer 0.10 plugin for the Pulse...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jeffrey C. Ollie
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2007-01-25 16:04 UTC by Matěj Cepl
Modified: 2018-04-11 16:44 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-01-29 10:11:35 UTC
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Matěj Cepl 2007-01-25 16:04:18 UTC
Spec URL: http://www.ceplovi.cz/matej/progs/rpms/gst-pulse.spec
SRPM URL: http://www.ceplovi.cz/matej/progs/rpms/gst-pulse-0.9.4-0.mc.1.src.rpm
Description: 
gst-pulse is a GStreamer 0.10 plugin for the PulseAudio sound server.

Comment 1 Jeffrey C. Ollie 2007-01-25 16:33:45 UTC
I'll have a review in a bit, but perhaps this package would be better named
gstreamer-plugins-pulse?  That would match better with other gstreamer plugins
(except gnonlin AFAIK).

Comment 2 Matthias Clasen 2007-01-25 17:39:09 UTC
I agree that gstreamer-plugins-pulse would be a better package name.

Comment 3 Matěj Cepl 2007-01-25 18:15:57 UTC
Well, it is ONE plugin, but I have changed the name anyway, new URLs are
SPEC: http://www.ceplovi.cz/matej/progs/rpms/gstreamer-plugins-pulse.spec
SRPM:
http://www.ceplovi.cz/matej/progs/rpms/gstreamer-plugins-pulse-0.9.4-0.mc.2.src.rpm

Comment 4 Jeffrey C. Ollie 2007-01-25 18:47:59 UTC
Here are a few things to fix, and then I'll do the full review:

* Remove the .la files in %install
* Add --disable-static to the %configure command to not build and
  package static library
* Need to add "gstreamer-plugins-base-devel" as a build requirement.
* Release field should be "2{%?dist}".


Comment 5 Matthias Clasen 2007-01-25 23:48:26 UTC
I independently made exactly the same changes to the spec file, and after adding
--disable-static, rpmlint is happy.

Comment 6 Jeffrey C. Ollie 2007-01-26 01:15:15 UTC
Please post the updated spec/srpm so that I can finish the review...

Comment 8 Jeffrey C. Ollie 2007-01-27 06:13:58 UTC
* source files match upstream

$ md5sum gst-pulse-0.9.4.tar.gz*
7c60018e8b9ce7f62c7078bee5851f07  gst-pulse-0.9.4.tar.gz
7c60018e8b9ce7f62c7078bee5851f07  gst-pulse-0.9.4.tar.gz.1

$ sha1sum gst-pulse-0.9.4.tar.gz*
6f8fd58d3f3a17e4c5e2c8b59d4ab06453082240  gst-pulse-0.9.4.tar.gz
6f8fd58d3f3a17e4c5e2c8b59d4ab06453082240  gst-pulse-0.9.4.tar.gz.1

* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (fc6, i386)
* package installs properly. 
! rpmlint says:

E: gstreamer-plugins-pulse script-without-shebang
/usr/share/doc/gstreamer-plugins-pulse-0.9.4/README.Fedora

Either specify a mode ("install -m 644 -p") or use "cp" to get the
file into the build directory.

E: gstreamer-plugins-pulse description-line-too-long gstreamer-plugins-pulse is
a GStreamer 0.10 plugin for the PulseAudio sound server.
E: gstreamer-plugins-pulse description-line-too-long gstreamer-plugins-pulse is
a GStreamer 0.10 plugin for the PulseAudio sound server.

I don't think that it's necessary to have the name of the package in
the description, I'd get rid of it.

* %check is not present; There is no test code in the districution.
* no shared libraries are present
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers
* no unversioned .so file
* no pkconfig file
* no libtool .la droppings.
* Final requirements are:

libc.so.6  
libc.so.6(GLIBC_2.0)  
libc.so.6(GLIBC_2.1.3)  
libc.so.6(GLIBC_2.4)  
libdl.so.2  
libglib-2.0.so.0  
libgmodule-2.0.so.0  
libgobject-2.0.so.0  
libgstaudio-0.10.so.0  
libgstpulse.so.0  
libgstreamer-0.10.so.0  
libgthread-2.0.so.0  
libm.so.6  
libpthread.so.0  
libpulse.so.0  
libxml2.so.2  
libz.so.1  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rtld(GNU_HASH)  

* The final provides are:

libgstpulse.so.0  
gstreamer-plugins-pulse = 0.9.4-3.fc6

The rpmlint errors are minor, you can fix them up after importing (but
before issuing a build please).  This package is APPROVED.


Comment 9 Lubomir Kundrak 2008-04-19 13:39:42 UTC
Package Change Request
======================
Package Name: foobar
New Branches: F-9 EL-5
Owner of EL-5 branch: lkundrak

Comment 10 Kevin Fenzi 2008-04-22 17:01:38 UTC
I assume you meant gstreamer-plugins-pulse for the package here, not "foobar".

cvs done.


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