Bug 170296 - Review Request: qalculate-kde - qt gui frontends to qalculate
Review Request: qalculate-kde - qt gui frontends to qalculate
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul Howarth
David Lawrence
http://qalculate.sourceforge.net/
:
Depends On: 169971
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-10-10 12:14 EDT by Deji Akingunola
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: 2005-10-17 16:23:45 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Spec file cleanup (3.80 KB, patch)
2005-10-13 12:42 EDT, Paul Howarth
no flags Details | Diff

  None (edit)
Description Deji Akingunola 2005-10-10 12:14:11 EDT
Spec Name or Url: ftp://czar.eas.yorku.ca/pub/qalculate/qalculate-kde.spec
SRPM Name or Url: ftp://czar.eas.yorku.ca/pub/qalculate/qalculate-kde-1.8.1.1-1.src.rpm
Description: QT/KDE gui frontends to Qalculate! desktop calculator.
Comment 1 Deji 2005-10-11 12:59:32 EDT
Package have been updated to the new released version, and desktop file installed.
ftp://czar.eas.yorku.ca/pub/qalculate/qalculate-kde.spec
ftp://czar.eas.yorku.ca/pub/qalculate/qalculate-kde-1.8.2-1.src.rpm.
Comment 2 Paul Howarth 2005-10-13 12:42:22 EDT
Created attachment 119927 [details]
Spec file cleanup

First pass (not a full review):

URL is actually:
ftp://czar.eas.yorku.ca/pub/qalculate/qalculate-kde-0.8.2-1.src.rpm

* libqalculate buildreq should be libqalculate-devel, not libqalculate

* qt-devel >= 3.0 buildreq is pulled in by kdelibs-devel buildreq (at least for
all distros supported by Fedora Extras)

* cln-devel, libxml2-devel, glib2-devel buildreqs are pulled in with
libqalculate-devel buildreq

* explicit Requires: of libqalculate not needed because RPM will generate the
library dependency automatically

* I think you need to use --disable-rpath with %configure to prevent rpath
generation

* only one desktop file should be included - use --delete-original with
desktop-file-install

* why the complexity of a loop in adding the four text files as %doc (as
opposed to just having "%doc AUTHORS ChangeLog COPYING TODO" in the files
list)?

* %clean section missing from spec file

* the TODO file is empty and shouldn't be included.

Attached patch addresses the above issues. Patched package builds OK in mock
and appears to run (and the documentation works too...).
Comment 3 Deji Akingunola 2005-10-13 18:03:13 EDT
Thanks Paul for the patch(es) and review. New files with corrections uploaded.
ftp://czar.eas.yorku.ca/pub/qalculate/qalculate-kde.spec
ftp://czar.eas.yorku.ca/pub/qalculate/qalculate-kde-0.8.2-2.src.rpm.
Comment 4 Paul Howarth 2005-10-14 11:39:57 EDT
Review:

- rpmlint nearly clean - see below
- package and spec file naming OK
- package meets guidelines
- license is GPL, matches spec, text included
- spec file written in English and is legible
- sources match upstream
- package builds OK in mock for FC4 (i386)
- BR's OK
- %find_lang used to handle locale data properly
- HTML documentation also handled properly
- no libraries, pkgconfigs or subpackages to worry about
- not relocatable
- no directory ownership or permissions issues
- no duplicate files
- %clean section present and correct
- macro usage is consistent
- code, not content
- docs not excessively large
- docs don't affect runtime, except for online help not working if --excludedocs
is used
- desktop file installed properly
- package appears to function correctly
- no scriptlets
- all previous issues addressed

Notes:

- rpmlint output:
  W: qalculate-kde dangling-symlink /usr/share/doc/HTML/en/qalculate_kde/common
/usr/share/doc/HTML/en/common
  W: qalculate-kde symlink-should-be-relative
/usr/share/doc/HTML/en/qalculate_kde/common /usr/share/doc/HTML/en/common
  this is common with KDE apps and has been OK-ed before:
  http://www.redhat.com/archives/fedora-extras-list/2005-July/msg01495.html

Approved.

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