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 169971 - Review Request: libqalculate - Multi-purpose calculator library
Summary: Review Request: libqalculate - Multi-purpose calculator library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Paul Howarth
QA Contact: David Lawrence
URL: http://qalculate.sourceforge.net/
Whiteboard:
: 169974 (view as bug list)
Depends On:
Blocks: FE-ACCEPT 169972 170296
TreeView+ depends on / blocked
 
Reported: 2005-10-05 23:52 UTC by Deji Akingunola
Modified: 2008-04-07 08:16 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-10-17 20:22:31 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Patch addressing review issues (deleted)
2005-10-10 17:18 UTC, Paul Howarth
no flags Details | Diff
Split off qalculate subpackage and update to 0.8.2 (deleted)
2005-10-11 12:28 UTC, Paul Howarth
no flags Details | Diff

Description Deji Akingunola 2005-10-05 23:52:13 UTC
Spec Name or Url: ftp://czar.eas.yorku.ca/pub/qalculate/libqalculate.spec
SRPM Name or Url: ftp://czar.eas.yorku.ca/pub/qalculate/libqalculate-0.8.1.2-1.src.rpm
Description: Qalculate! is a multi-purpose desktop calculator for GNU/Linux. It is small and simple to use but with much power and versatility underneath. Features include customizable functions, units, arbitrary precision, plotting, and a user-friendly interface (KDE or GTK+).

Comment 1 Paul Howarth 2005-10-10 13:46:37 UTC
*** Bug 169974 has been marked as a duplicate of this bug. ***

Comment 2 Paul Howarth 2005-10-10 17:17:22 UTC
Good:

- package and spec naming OK
- package meets most guidelines
- license is GPL
- spec file written in English and is legible
- sources match upstream
- package builds OK on FC4 and in mock for rawhide (i386)
- proper use of %find_lang for locales
- not relocatable
- no directory ownership or permissions issues
- no duplicate files
- %clean section present and correct
- macro usage is consistent
- code, not content
- no large docs
- docs don't affect runtime
- libtool archive not included
- no desktop file needed
- qalc seems to run ok, but I only played very briefly with it

Needswork:

- rpmlint not clean, mainly due to not splitting off header files etc. into a
  separate -devel subpackage
- BuildReqs need a bit of work (see below)
- ldconfig not run in %post/%postun
- package includes static libraries (deprecated in Fedora)
- http://qalculate.sourceforge.net/downloads.html suggests that libxml2 >=
  2.3.8 is a buildreq
- the program will include readline/ncurses support if available at build
  time, so readline-devel & ncurses-devel should be added as buildreqs
- explicit glib2 & libxml2 deps are redundant and should be removed
- the Makefile includes DESTDIR support, which I believe is preferred to
  using %makeinstall
- the README file just includes a URL to visit so isn't really worth including
- the AUTHORS and TODO files probably are worth including as %doc
- license text in COPYING should be included as %doc

Suggestions:

- perhaps split off a separate package for the text-mode qalc program itself
  (maybe called qalculate?)

I'll attach a patch addressing the Needswork issues.


I haven't reviewed a library before so anyone more experienced with those can
please take a look too...


Comment 3 Paul Howarth 2005-10-10 17:18:13 UTC
Created attachment 119779 [details]
Patch addressing review issues

Comment 4 Deji Akingunola 2005-10-10 18:17:59 UTC
Updated spec file and .srpm here 
Spec Name or Url: ftp://czar.eas.yorku.ca/pub/qalculate/libqalculate.spec
SRPM Name or Url:
ftp://czar.eas.yorku.ca/pub/qalculate/libqalculate-1.8.1.2-2.src.rpm

I'm sure though if another package needs to be spin-off from this.

Thanks Paul for the review and the patch.

Deji


Comment 5 Michael Schwendt 2005-10-10 21:56:49 UTC
What I find odd is that the package is called "libqalculate", but that
the package description reads like it's an application
(quote: a "multi-purpose desktop calculator").

It is commonly considered bad taste to repeat the product name in
the "Summary:" line (better: "Summary: Multi-purpose calculator library"
or similar).

libcalculate-devel contains a pkg-config file which links -lgmp,
so the package should "Requires: gmp-devel".


Comment 6 Paul Howarth 2005-10-11 07:02:24 UTC
(In reply to comment #5)
> What I find odd is that the package is called "libqalculate", but that
> the package description reads like it's an application
> (quote: a "multi-purpose desktop calculator").

I agree, which is why I suggested a separate (sub)package for qalc itself.

> It is commonly considered bad taste to repeat the product name in
> the "Summary:" line (better: "Summary: Multi-purpose calculator library"
> or similar).

Agreed.

> libcalculate-devel contains a pkg-config file which links -lgmp,
> so the package should "Requires: gmp-devel".

gmp-devel is a dep of cln-devel, which is already a dep of libqalculate-devel.


Comment 7 Michael Schwendt 2005-10-11 09:51:58 UTC
> gmp-devel is a dep of cln-devel

Ah, okay, never mind. Your intention is to optimise the dependency chain.
On the contrary, I don't mind if a direct dependency of a file in the
package is reflected in a direct "Requires". Matter of taste. No big
issue.

Comment 8 Paul Howarth 2005-10-11 12:28:35 UTC
Created attachment 119800 [details]
Split off qalculate subpackage and update to 0.8.2

Attached patch updates spec for new 0.8.2 release and splits off the actual
calculator program into a separate package, with sensible descriptions for each
package.

Comment 9 Deji Akingunola 2005-10-11 15:03:14 UTC
I've implemented changes suggested by Michael and Paul; new spec file and srpm
available at;
ftp://czar.eas.yorku.ca/pub/qalculate/libqalculate.spec
ftp://czar.eas.yorku.ca/pub/qalculate/libqalculate-0.8.2-1.src.rpm

Thanks Michael and Paul.

Comment 10 Paul Howarth 2005-10-11 15:44:14 UTC
I'm happy enough with this to approve it now. Any more comments on it Michael
(or anyone else)?

Are you already a Fedora Extras Contributor, or are you looking for a sponsor?

Comment 11 Deji 2005-10-11 17:01:35 UTC
(In reply to comment #10)
> I'm happy enough with this to approve it now. Any more comments on it Michael
> (or anyone else)?
> 
> Are you already a Fedora Extras Contributor, or are you looking for a sponsor?

No, I'm not a Fedora Extras Contributor yet, and i'll like to have a sponsor.
Thanks.

Comment 12 Paul Howarth 2005-10-14 14:52:38 UTC
Have you been through the CLA part of the Fedora Extras contributors system yet?
If not, do that. Then apply for membership of the cvsextras group and let me
know what your account name is and I'll sponsor you.

Comment 13 Deji Akingunola 2005-10-14 16:28:40 UTC
(In reply to comment #12)
> Have you been through the CLA part of the Fedora Extras contributors system yet?
> If not, do that. Then apply for membership of the cvsextras group and let me
> know what your account name is and I'll sponsor you.

I've just gone through the process now. My account name is 'deji'.
Thanks.


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