Bug 191671 - Review Request: serpentine
Review Request: serpentine
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-14 21:17 EDT by Sindre Pedersen Bjørdal
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-06-28 11:11:20 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


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

  None (edit)
Description Sindre Pedersen Bjørdal 2006-05-14 21:17:32 EDT
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 15:58:41 EDT
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 18:22:26 EDT
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 19:21:52 EDT
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 19:29:54 EDT
(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 16:57:55 EDT
I'm jumping in for a formal review. Stay tuned.
Comment 6 Christoph Wickert 2006-05-19 08:37:23 EDT
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 08:40:04 EDT
Created attachment 129598 [details]
Patch for serpentine.spec

diff -u serpentine-0.6.4.spec serpentine.spec
Comment 8 Christoph Wickert 2006-05-19 08:46:41 EDT
(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 11:53:49 EDT
$ 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 17:49:31 EDT
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 11:11:20 EDT
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.