Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 191671 - Review Request: serpentine
Summary: Review Request: serpentine
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christoph Wickert
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-05-15 01:17 UTC by Sindre Pedersen Bjørdal
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-06-28 15:11:20 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Patch for serpentine.spec (2.65 KB, patch)
2006-05-19 12:40 UTC, Christoph Wickert
no flags Details | Diff

Description Sindre Pedersen Bjørdal 2006-05-15 01:17:32 UTC
Spec URL: http://folk.ntnu.no/sindrb/packages/serpentine.spec
SRPM URL: http://folk.ntnu.no/sindrb/packages/serpentine-0.6.4-1.src.rpm

Description:

Serpentine is an application for writing CD-Audio discs. 
It aims for simplicity, usability and compatibility

Comment 1 Gérard Milmeister 2006-05-16 19:58:41 UTC
A few preliminary comments:
* These directories should be owned, otherwise they hang around after
  uninstalling:
  /usr/lib/python2.4/site-packages/serpentine
  /usr/share/serpentine
* The russian entry in the .desktop file doesn't seem to be UTF-8...


Comment 2 Sindre Pedersen Bjørdal 2006-05-16 22:22:26 UTC
How can I deal with the russian entry issue? The ru.po file claims to be utf-8.

Comment 3 Jason Tibbitts 2006-05-16 23:21:52 UTC
I'm pretty sure it's not UTF-8.  I ran through a bunch of encodings and didn't
see one that would produce meaningful output when viewed on a Unicode-capable
terminal.

I would ask upstream.  If you have no luck there, you could always remove the
Russian translation and work to get it in later.

Comment 4 Gérard Milmeister 2006-05-16 23:29:54 UTC
(In reply to comment #2)
> How can I deal with the russian entry issue? The ru.po file claims to be utf-8.
I now tested it running with "LANG=ru_RU serpentine": It is certainly not utf-8.
Only the standard menus have correct cyrillic letters.
This is however not a blocker, since it is not your job as a packager to ensure
that translations are correct. You should simply notify upstream about it.

Another point from http://fedoraproject.org/wiki/Packaging/Python:
The python-abi req is not needed on FC-4 upwards anymore.

Comment 5 Christoph Wickert 2006-05-18 20:57:55 UTC
I'm jumping in for a formal review. Stay tuned.

Comment 6 Christoph Wickert 2006-05-19 12:37:23 UTC
Is there a particular reason not to use serpentine-0.6.91?

http://developer.berlios.de/project/shownotes.php?group_id=3081&release_id=9269

0.6.91 introduces support for gstreamer-1.0 and fixes the issuse with the
russian desktop.file and translations. Unfortunately German translations were
dropped :(, maybe I'm going to update them.

Attaching a patch. This patch also fixes the following issues:
- own /usr/lib/python2.4/site-packages/serpentine and
  /usr/share/serpentine (comment #1) 
- drop python-abi Requires: (comment # 4)
- drop serpentine-0.6.4-desktop.patch and use sed instead
- description now ends with a dot: "...simplicity, usability and compatibility."
  BTW: IMHO description could be a little mor elaborate, something like:
"Serpentine is an application for writing CD-Audio discs. 
It aims for simplicity, usability and compatibility and accepts a big range of
audio (and video) formats thanks to the excelent GStreamer framework. It also
tries to integrate well with other applications, accepting full Drag N Drop
from applications like Nautilus, Rhythmbox and even Firefox."
(parts taken from http://gnomefiles.org/app.php?soft_id=907)
- require gstreamer-python instead of gstreamer08-python
- remove-category X-Ximian-Main from fedora-serpentine.desktop

A minor note: Calling update-desktop-database in post and postun was not
necessary, because the desktop entry did not contain a mime type. 0.6.91 has a
mime type, so we do need it now.

Take what you need from my patch and update your package please. I'm going do do
a complete review then. From what I've seen everything looks fine, package
builds in mock and works well. I've successfully burned a couple of audio discs,
in fact I'm using serpentine for a long time and rolled my own package. Nice to
see somebody is willing to maintain it for extras. :-)

Comment 7 Christoph Wickert 2006-05-19 12:40:04 UTC
Created attachment 129598 [details]
Patch for serpentine.spec

diff -u serpentine-0.6.4.spec serpentine.spec

Comment 8 Christoph Wickert 2006-05-19 12:46:41 UTC
(In reply to comment #7)

Sorry, I just realized there'S a typo in my patch:
"--remove-cetagory" needs to be
"--remove-category"


Comment 10 Christoph Wickert 2006-05-19 15:53:49 UTC
$ md5sum serpentine-0.6.91-1.src.rpm
5965ec6a4622440337452084f79c5c59  serpentine-0.6.91-1.src.rpm

REVIEW:

- rpmlint clean:
$ ls *.rpm
serpentine-0.6.91-1.noarch.rpm  serpentine-0.6.91-1.src.rpm
$ rpmlint *.rpm ; echo $?
0

- package and spec naming OK
- package meets guidelines
- license is GPL, matches license field spec
- license both included in source and %doc
- spec file written in English and is legible
- sources match upstream
- package builds OK on FC5 (noarch) and in FC5 and rawhide mock (noarch)
- BR's OK, on duplicates, no exeptions
- locales handled correctly
- no libraries to worry about
- not relocatable
- no directory ownership issues
- no duplicates in %files
- permissions OK, correct %defattr
- %clean section present and correct
- macro usage consistent
- code, not content
- no large docs
- docs don't affect runtime
- no pkgconfigs to worry about
- no devel sub package needed
- desktop file OK and properly installed
- scriptlets match examples from wiki

APPROVED

Comment 11 Christoph Wickert 2006-06-14 21:49:31 UTC
Just a reminder that this report should probably be closed, package is in CVS,
owners.list and in the FE repos.

BTW. 0.7 is out. :-)

Comment 12 Christoph Wickert 2006-06-28 15:11:20 UTC
Sindre, please close you reviews if your package becomes available.

Closing.


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