Bug 243576 - Review Request: gst-inspector - An introspection data viewer for the GStreamer multimedia framework
Review Request: gst-inspector - An introspection data viewer for the GStreame...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-09 22:09 EDT by Jeffrey C. Ollie
Modified: 2007-11-30 17:12 EST (History)
2 users (show)

See Also:
Fixed In Version: 0.3-5.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-11-20 12:58:57 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jspaleta: fedora‑review+
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jeffrey C. Ollie 2007-06-09 22:09:43 EDT
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 13:42:56 EDT
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 15:02:59 EDT
Ping
Are you still in the market for a formal review of this?
Comment 4 Jef Spaleta 2007-10-23 16:19:28 EDT
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 07:34:03 EDT
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 18:31:35 EDT
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 09:12:07 EDT
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@ocjtech.us> - 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 01:09:16 EDT
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 08:01:49 EDT
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 00:59:05 EDT
Packages imported, built, and pushed requested.
Comment 11 Fedora Update System 2007-11-06 11:06:01 EST
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 18:58:54 EST
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 12:58:55 EST
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 13:02:17 EST
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.