Bug 243576 - Review Request: gst-inspector - An introspection data viewer for the GStreamer multimedia framework
Summary: Review Request: gst-inspector - An introspection data viewer for the GStreame...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-06-10 02:09 UTC by Jeffrey C. Ollie
Modified: 2007-11-30 22:12 UTC (History)
2 users (show)

Fixed In Version: 0.3-5.fc7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-11-20 17:58:57 UTC
Type: ---
Embargoed:
jspaleta: fedora-review+
wtogami: fedora-cvs+


Attachments (Terms of Use)

Description Jeffrey C. Ollie 2007-06-10 02:09:43 UTC
Spec URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/gst-inspector-0.1-1.fc6.spec
SRPM URL: http://repo.ocjtech.us/misc/fedora/6/SRPMS/gst-inspector-0.1-1.fc6.src.rpm
Description:
GStreamer Inspector is an introspection data viewer for the GStreamer
multimedia framework. Developers of GStreamer applications can use it
to find out about the various elements available in the system.

It can be thought of as a graphical variant of the gst-inspect
program, which currently serves as the canonical source of
introspection information for many developers.

Comment 1 Philippe Valembois 2007-06-21 17:42:56 UTC
Seems good to me :
built on Mock
rpmlint is silent
Package Guidelines seems to be followed except on two points :
"If the package doesn't already include and install its own .desktop file, you
need to make your own, and include it as a Source"
"If upstream uses <vendor_id>, leave it intact, otherwise use fedora as <vendor_id>"



Comment 3 Jef Spaleta 2007-10-23 19:02:59 UTC
Ping
Are you still in the market for a formal review of this?

Comment 4 Jef Spaleta 2007-10-23 20:19:28 UTC
One quick note you need to update the license tag in the spec file to read
GPLv2+    since the text of the header in the source files reads 
"version 2 of the License, or (at your option) any later version."

The licensing guidelines changed since the ticket was opened. If you haven't
done so you should review the new licensing guidance.

Don't spin up a new srpm and spec for just this change. I'll make a formal
review and you can spin a new spec and srpm with all changes.

-jef 

Comment 5 Jeffrey C. Ollie 2007-10-24 11:34:03 UTC
Yeah, I'd love to have a formal review...  I noticed that there was a newer
version out, so I updated the package and fixed the license tag.  It's GPLv3+ now...

Spec URL: http://repo.ocjtech.us/misc/fedora/7/SRPMS/gst-inspector-0.3-1.fc7.spec
SRPM URL: http://repo.ocjtech.us/misc/fedora/7/SRPMS/gst-inspector-0.3-1.fc7.src.rpm


Comment 6 Jef Spaleta 2007-10-24 22:31:35 UTC
The GOOD:
rpmlint runs clean
fits naming guidelines
specfile naming good
Licensing good: GPLv3+  and tag matches
Copying file included in docs as required
spec written in english-ese and is legible
md5sum for source matches upstream location:
  69bcc7a8939582ebdb806579947b316c  gst-inspector-0.3.tar.gz
builds noarch in mock against f7 and program runs
buildrequires look sane
no locales to worry about
no libs installed that need ldconfig
relocatable n/a
no duplicate file entries
owns all directories it creates
clean section is good
macro usage consistent
no content issues
no need for a docs subpackage
all doc files are non-essential
no need for -devel package
no static libs
install section looks good
desktop file included for gui application.

The BAD:
directory ownership chain issue
Need to require hicolor-icon-theme  since you are installing icons in
/usr/share/icons/hicolor/ 

need a postun section for the icon cache updating. You have the post  you just
need the corresponding postun as well:

%postun
touch --no-create %{_datadir}/icons/hicolor
if [ -x %{_bindir}/gtk-update-icon-cache ]; then
  %{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || :
fi


The UGLY:
Are you sure you want to put gst-inspector in audio/video?  I would have thought
this would be better in Programming menu since this isn't so much an aid to
end-users but an aid to people developing gst based scripts and applications.

-jef


Comment 7 Jeffrey C. Ollie 2007-10-25 13:12:07 UTC
Thanks for the review!  I've fixed up the problems that were identified and
posted new spec/SRPM:

Spec URL: http://repo.ocjtech.us/misc/fedora/7/SRPMS/gst-inspector-0.3-2.fc7.spec
SRPM URL: http://repo.ocjtech.us/misc/fedora/7/SRPMS/gst-inspector-0.3-2.fc7.src.rpm

* Thu Oct 25 2007 Jeffrey C. Ollie <jeff> - 0.3-2
- Add postun and redo post section to match
- Require hicolor-icons-theme
- Change category for desktop entry


Comment 8 Jef Spaleta 2007-10-26 05:09:16 UTC
Approved

okay 0.3.2 looks good to me.. except one thing.
Please use %{SOURCE1} instead of %{S:1} for legibility in the spec before you
commit to cvs. 

-jef



Comment 9 Jeffrey C. Ollie 2007-10-26 12:01:49 UTC
Thanks for the review!  I'll change that macro as requested.

Package Change Request
======================
Package Name: gst-inspector
New Branches: F-7 F-8 devel
Updated Fedora Owners: jcollie
Updated Description: An introspection data viewer for the GStreamer multimedia
framework


Comment 10 Jeffrey C. Ollie 2007-11-03 04:59:05 UTC
Packages imported, built, and pushed requested.

Comment 11 Fedora Update System 2007-11-06 16:06:01 UTC
gst-inspector-0.3-5.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update gst-inspector'

Comment 12 Fedora Update System 2007-11-09 23:58:54 UTC
gst-inspector-0.3-5.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update gst-inspector'

Comment 13 Fedora Update System 2007-11-20 17:58:55 UTC
gst-inspector-0.3-5.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 14 Fedora Update System 2007-11-20 18:02:17 UTC
gst-inspector-0.3-5.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.


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