Bug 461011 - Review Request: kde-plasma-lancelot - An alternative application launcher
Review Request: kde-plasma-lancelot - An alternative application launcher
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Kofler
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-03 10:01 EDT by Ngo Than
Modified: 2008-11-11 19:07 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-11-11 19:07:52 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)

  None (edit)
Description Ngo Than 2008-09-03 10:01:05 EDT
Spec URL: http://than.fedorapeople.org/kde-plasma-lancelot.spec
SRPM URL: http://than.fedorapeople.org/kde-plasma-lancelot-1.0-1.fc9.src.rpm
Description: An alternative application launcher.
Comment 1 Kevin Kofler 2008-09-04 02:34:21 EDT
Reviewing this.
Comment 2 Kevin Kofler 2008-09-09 08:45:34 EDT
rpmlint output (reordered for clarity):

> kde-plasma-lancelot.src: W: summary-ended-with-dot An alternative application launcher.
> kde-plasma-lancelot.i386: W: summary-ended-with-dot An alternative application launcher.

Summary should not end with a dot, please remove the dot. :-)

> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/widgets/HoverIcon.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/layouts/ColumnLayout.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/widgets/Widget.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/Global.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/models/PassagewayViewModels.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/widgets/ResizeBordersPanel.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/lancelot_export.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/widgets/Panel.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/widgets/ExtenderButton.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/lib/liblancelot.so
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/widgets/WidgetPositioner.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/widgets/ActionListView.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/layouts/CardLayout.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/widgets/BasicWidget.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/widgets/PassagewayView.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/models/ActionListViewModels.h
> kde-plasma-lancelot.i386: W: devel-file-in-non-devel-package /usr/include/kde4/lancelot/lancelot.h

If we want to support building things against liblancelot, these need to go into a kde-plasma-lancelot-devel, otherwise we should just rm -rf them.

> kde-plasma-lancelot.i386: E: library-without-ldconfig-postin /usr/lib/liblancelot.so.0.5.0

Missing:
%post -p /sbin/ldconfig

> kde-plasma-lancelot.i386: E: library-without-ldconfig-postun /usr/lib/liblancelot.so.0.5.0

Missing:
%postun -p /sbin/ldconfig

Can you please fix these first, before I go through my manual checklist?
Comment 3 Kevin Kofler 2008-09-09 08:52:40 EDT
I'd also suggest pointing URL to http://www.kde-apps.org/content/show.php/Lancelot?content=87914 instead of the SVN branch.
Comment 4 Ngo Than 2008-09-09 09:12:01 EDT
i uploaded new spec and srpm that fixes the above issue.
Comment 5 Kevin Kofler 2008-09-09 12:04:41 EDT
MUST Items:
+ rpmlint output OK (no warnings or errors)
+ named and versioned according to the Package Naming Guidelines
+ spec file name matches base package name
+ Packaging Guidelines:
  ! License is listed as GPLv3+, is actually GPLv2+
  + No known patent problems
  + No emulator, no firmware, no binary-only or prebuilt components
  + Complies with the FHS
  + proper changelog, tags, BuildRoot, BuildRequires, Summary, Description
  ! Requires: kdelibs4 >= 4 should be >= 4.1 if mentioned, but it isn't needed at all, as the autoreqs take care of it (libplasma.so.2, i.e. kdebase-workspace-libs 4.1 is automatically dragged in, and that Requires a kdelibs4 >= 4.1 already)
  + no non-UTF-8 characters
  + upstream TODO is included, no other documentation
  + RPM_OPT_FLAGS are used (%cmake_kde4 macro)
  + debuginfo package is valid
  + no static libraries nor .la files
  + no duplicated system libraries
  + no rpaths
  + no configuration files, so %config guideline doesn't apply
  + no init scripts, so init script guideline doesn't apply
  + no GUI executables (in the proper sense), so no .desktop file needed
  + ... and thus no desktop-file-install needed either
  + no timestamp-clobbering file commands
  + _smp_mflags used
  ! scriptlets:
    mkdir %{buildroot} fails in Rawhide because the parent directory of the new RPM's new buildroot is not getting created by mock (unless that has been fixed), it's probably easiest to just omit it and let make install create the directories recursively. (The buildroot used by the Rawhide RPM is secure against race conditions, by the way, it's under the user's home directory.)
  + not a web application, so web application guideline doesn't apply
  + no conflicts
+ complies with all the legal guidelines
! COPYING not packaged as %doc
! source does not match upstream 1.0 release at:
  http://sourceforge.net/project/showfiles.php?group_id=238004&package_id=289266&release_id=622496
  The source tarball is polluted with .svn directories, probably a checkout rather than an export. Please use the upstream tarball instead!
! source tarball should be specified using full URL according to the guidelines for SourceForge-hosted packages
  https://fedoraproject.org/wiki/Packaging/SourceURL
  -> http://downloads.sourceforge.net/lancelot-menu/lancelot-1.0.tar.bz2
+ builds on at least one arch (F9 Koji scratch build on all arches)
+ no non-working arches, so no ExcludeArch needed
+ no missing BuildRequires (builds in Koji)
+ no translations, so translation/locale guidelines don't apply
+ ldconfig correctly called in %post and %postun
+ package not relocatable
! unowned directory %{_kde4_appsdir}/desktoptheme/default/lancelot/
  -> please remove the * from %{_kde4_appsdir}/desktoptheme/default/lancelot/*
  (other than that, no package-specific directories which would have to be owned, doesn't own directories owned by another package)
+ no duplicate files in %files
+ permissions correct, defattr used correctly
+ %clean section present and correct
+ macros used where possible
+ no non-code content
+ no large documentation files, so no -doc package needed
+ no %doc files required at runtime
+ no header files which would need to be in a -devel subpackage
+ no static libraries, so no -static package needed
+ no .pc files, so no Requires: pkgconfig needed
+ no devel symlinks which would need to be in a -devel subpackage
+ plugins in %{_kde4_libdir}/kde4/ are correctly NOT in a -devel subpackage
+ no -devel package, so "-devel should require %{name} = %{version}-%{release}" is irrelevant
+ no .la files
+ no GUI executables, so no .desktop file needed
+ buildroot is deleted at the beginning of %install
+ all filenames are valid UTF-8

SHOULD Items:
+ license already included upstream
+ no translations for description and summary provided by upstream
+ package builds in mock, on all architectures (F9 Koji scratch build)
* not tested functionality (will do once the other stuff is fixed)
+ 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


Please fix the following MUST FIX items:
* License is listed as GPLv3+, is actually GPLv2+
* remove unneeded Requires: kdelibs4 >= 4, a more precise dependency is already implied automatically
* remove the explicit mkdir %{buildroot} (at least until the mock used by Koji gets fixed)
* add COPYING as %doc
* use the upstream tarball from: http://sourceforge.net/project/showfiles.php?group_id=238004&package_id=289266&release_id=622496
* Source: should have full URL: http://downloads.sourceforge.net/lancelot-menu/lancelot-1.0.tar.bz2
* remove the * from %{_kde4_appsdir}/desktoptheme/default/lancelot/* (to own the directory)

Once these are fixed, I'll do a short test to make sure it actually works and then I'll approve it.
Comment 6 Ngo Than 2008-09-09 16:05:20 EDT
it's fixed now. Please take a look at new specfile and srpms. Thanks
Comment 7 Kevin Kofler 2008-09-09 16:23:44 EDT
All objections addressed. Tarball checksums (matching upstream):
MD5SUM: 20ad6b20817ac0f0e6903930c27e7d85
SHA1SUM: e1dc6ddee25de98ba7ba81b953486c60406d1362

I tested this quickly, appears to work fine.

APPROVED
Comment 8 Ngo Than 2008-09-09 16:38:00 EDT
New Package CVS Request
=======================
Package Name: kde-plasma-lancelot
Short Description: An alternative application launcher
Owners: than
Branches: F-9, F-10
InitialCC: than
Comment 9 Kevin Fenzi 2008-09-09 19:42:11 EDT
cvs done.
Comment 10 Ngo Than 2008-09-10 10:06:00 EDT
packages are already commited in CVS.
Comment 11 Kevin Kofler 2008-11-11 19:07:52 EST
That stuff has been built and pushed for a while.

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