Bug 432607 - Review Request: kmid - A midi/karaoke player for KDE
Review Request: kmid - A midi/karaoke player for KDE
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Kofler
Fedora Extras Quality Assurance
i
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-13 05:47 EST by Sebastian Vahl
Modified: 2008-03-04 04:46 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-03-04 04:46:43 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
remove-kmid-examples.sh (210 bytes, application/x-shellscript)
2008-03-03 16:16 EST, Kevin Kofler
no flags Details

  None (edit)
Description Sebastian Vahl 2008-02-13 05:47:03 EST
Spec URL: http://svahl.fedorapeople.org/kmid/kmid.spec
SRPM URL: http://svahl.fedorapeople.org/kmid/kmid-2.0-0.2.20080213.fc8.src.rpm
Description: 
KMid is a midi/karaoke file player, with configurable midi mapper, real 
Session Management, drag & drop, customizable fonts, etc. It has a very 
nice interface which let you easily follow the tune while changing the 
color of the lyrics.
It supports output through external synthesizers, AWE, FM and GUS cards.
It also has a keyboard view to see the notes played by each instrument.

Note for reviewers:
kmid has no official release of the KDE4 port yet. So the svn version is packaged here. But it should be as usuable as the KDE 3 version (last change was over one month ago). To create the svn checkout a modified script from upstream was used. There is also existing a short documentation about how to create a new tarball at your own.

rpmlint:
kmid.i386: W: dangling-symlink /usr/share/doc/HTML/en/kmid/common /usr/share/doc/HTML/en/common
kmid.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/kmid/common /usr/share/doc/HTML/en/common
kmid-devel.i386: W: no-documentation

The first two are normal for KDE packages. For the latter: The tarball doesn't include any development documentation.

scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=422448
Comment 1 Kevin Kofler 2008-02-23 00:44:05 EST
Hmmm, I can't download the files from the scratch build to run rpmlint on them, 
Koji errors. :-(
Comment 2 Sebastian Vahl 2008-02-23 15:55:28 EST
New scratch build (the old files seems to be deleted):
http://koji.fedoraproject.org/koji/taskinfo?taskID=464604
Comment 3 Kevin Kofler 2008-03-02 11:25:05 EST
MUST Items:
+ rpmlint output: as quoted by submitter, all harmless
! not entirely named and versioned according to the Package Naming Guidelines:
  Release should be 0.2.%{svn_date}svn%{?dist} instead of 0.2.
%{svn_date}%{?dist}
+ spec file name matches base package name
+ Packaging Guidelines:
  + License GPLv2+ OK, matches actual license
  + No known patent problems
  + No emulator, no firmware, no binary-only or prebuilt components
  + Complies with the FHS
  + proper changelog, tags, BuildRoot, Requires, BuildRequires, Summary, 
Description
  + no non-UTF-8 characters
  + relevant documentation included
  + RPM_OPT_FLAGS are used (%{cmake_kde4} macro)
  + debuginfo package is valid
  + no static libraries nor .la files
  + no duplicated system libraries
  + no rpaths on i386 and x86_64
  + no configuration files, so %config guideline doesn't apply
  + no init scripts, so init script guideline doesn't apply
  + GUI executable has .desktop file
  + .desktop file installed according to the guidelines (uses 
desktop-file-install)
  + no timestamp-clobbering file commands
  + _smp_mflags used
  + scriptlets are valid
  + not a web application, so web application guideline doesn't apply
  + no conflicts
+ complies with all the legal guidelines
+ license included as %doc
+ complies with all the legal guidelines
* skipping "source matches upstream" test because this is a SVN checkout
+ builds on at least one arch (F9 all arches Koji scratch build)
+ no non-working arches, so no ExcludeArch needed
+ all build dependencies listed in CMakeLists.txt as well as cmake itself are 
listed in BuildRequires
+ translations packaged according to the guidelines (%find_lang)
+ ldconfig correctly called in %post and %postun
+ package not relocatable
+ ownership correct (owns package-specific directories, doesn't own directories 
owned by another package)
+ no duplicate files in %files
! permissions: -devel package missing %defattr(-,root,root,-)
+ %clean section present and correct
+ macros used where possible
! non-code content: examples (examples/*.kar) have suspicious copyright status
  * DiesIrae.kar: Mozart, sequencing "(c) 1996 V. Palleschi", may or may not be 
OK, depending on whether V. Palleschi allowed this
  * Guantanamera.kar: Joseíto Fernández (1908-1979), almost certainly a 
copyright violation
  * MariaDeLasMercedes.kar: Concha Piquer (1908-1990), almost certainly a 
copyright violation
  * OFortuna.kar: from the Carmina Burana, music by Carl Orff (1895-1982), 
almost certainly a copyright violation
  IMHO we should probably remove the examples entirely (even from the tarball).
+ no large documentation files, so no -doc package needed
+ %doc files not required at runtime
+ all header files in -devel
+ no static libraries, so no -static package needed
+ no .pc files, so no Requires: pkgconfig needed
+ devel symlinks correctly in -devel
+ /usr/lib/kde4/*.so plugins (NOT symlinks) are correctly NOT in -devel
+ -devel Requires: %{name} = %{version}-%{release}
+ no .la files
+ GUI executable has .desktop file
+ buildroot is deleted at the beginning of %install
+ all filenames are valid UTF-8

SHOULD Items:
+ license included as %doc
+ no translations for description and summary provided by upstream
+ package builds in mock (Koji scratch build)
+ package builds on all architectures (Koji scratch build)
+ basic functionality tested on F8, works fine
+ scriptlets are sane
+ no subpackages other than -devel, so "Usually, subpackages other than devel 
should require the base package using a fully versioned dependency." is 
irrelevant
+ no .pc files, so "placement of .pc files" is irrelevant
+ no file dependencies


MUST FIX:
* add "svn" to the Release tag (after the date)
* add missing %defattr(-,root,root,-) in %files devel
* remove examples (suspicious copyright status)
Fix these and I'll approve this. One thing I'm not sure is how much sense it 
makes to provide kmid-devel, I don't think anything other than kmid itself 
actually uses the kmid libs.
Comment 4 Kevin Kofler 2008-03-03 16:16:42 EST
Created attachment 296679 [details]
remove-kmid-examples.sh

Here's a script to remove the (apparently copyright-encumbered) examples from
the kmid tarball (and also patch the top-level CMakeLists.txt not to try
recursing into the directory we removed).
Comment 5 Sebastian Vahl 2008-03-03 16:48:06 EST
Thanks for the script. I've included it as a comment in the spec file because 
it is short enough. For kmid-devel: I think I'll still package it because 
there may be some packages in the future which will need these files.

Spec URL: http://svahl.fedorapeople.org/kmid/kmid.spec
SRPM URL: http://svahl.fedorapeople.org/kmid/kmid-2.0-0.4.20080213.fc8.src.rpm

Changelog:
- add default %%defattr(-,root,root,-) also for devel files
- include "svn" in release tag
- remove non-code content with unknown copyright status from tarball and 
package
- remove KDE version from summary
Comment 7 Kevin Kofler 2008-03-03 16:54:28 EST
All issues addressed, looks OK now.
APPROVED
Comment 8 Sebastian Vahl 2008-03-03 16:59:38 EST
(In reply to comment #7)
> All issues addressed, looks OK now.
> APPROVED

Thanks!

New Package CVS Request
=======================
Package Name: kmid
Short Description: A midi/karaoke player for KDE 4
Owners: svahl,than,rdieter,kkofler,ltinkl
Branches: 
InitialCC: 
Cvsextras Commits: no

Comment 9 Kevin Fenzi 2008-03-03 21:58:24 EST
cvs done.
Comment 10 Sebastian Vahl 2008-03-04 04:46:43 EST
(In reply to comment #9)
> cvs done.

Thanks.

Imported and built: http://koji.fedoraproject.org/koji/buildinfo?buildID=41156


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