Bug 205041 - Review Request: tellico - collection manager
Summary: Review Request: tellico - collection manager
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kevin Fenzi
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-09-02 21:58 UTC by José Matos
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-19 23:40:40 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description José Matos 2006-09-02 21:58:04 UTC
Spec URL: http://www.fc.up.pt/pessoas/jamatos/fedora-extras/tellico.spec
SRPM URL: http://www.fc.up.pt/pessoas/jamatos/fedora-extras/tellico-1.2-1.src.rpm
Description:
Tellico is a collection manager for KDE. It includes default collections for
books, bibliographies, comic books, videos, music, coins, stamps, trading
cards, and wines, and also allows custom collections. Unlimited user-defined
fields are allowed. Filters are available to limit the visible entries by
definable criteria. Full customization for printing is possible through
editing the default XSLT file. It can import CSV, Bibtex, and Bibtexml and
export CSV, HTML, Bibtex, Bibtexml, and PilotDB. Entries may be imported
directly from Amazon.com.

Comment 1 Kevin Fenzi 2006-09-03 05:35:24 UTC
OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
a7ecc139d820279b0a89d8f594110094  tellico-1.2.tar.gz
a7ecc139d820279b0a89d8f594110094  tellico-1.2.tar.gz.1
See below - Package compiles and builds on at least one arch.
OK - BuildRequires correct
OK - Spec handles locales/find_lang
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package is a GUI app and has a .desktop file
OK - Package doesn't own any directories other packages own.
OK - No rpmlint output.

SHOULD Items:
OK - Should include License or ask upstream to include it.
See below - Should build in mock.
OK - Should have sane scriptlets.

Issues:

1. In your desktop-file-install you have --vendor="", but according
to guidelines that should be --vendor=fedora. Also, this causes
builds under mock to fail because the fedora-tellio.desktop file doesn't
exist.

2. INSTALL NEWS README can probibly all be dropped. INSTALL is the generic
auto* install document, NEWS and README are both of size 0.

3. The Summary is a bit generic:
Summary:        collection manager
Perhaps at least add that it's a KDE based collection manager?

4. rpmlint says:
W: tellico summary-not-capitalized collection manager
So, it should at least be "A KDE collection manager"


Comment 2 José Matos 2006-09-03 17:37:52 UTC
(In reply to comment #1)
> 
> 1. In your desktop-file-install you have --vendor="", but according
> to guidelines that should be --vendor=fedora. Also, this causes
> builds under mock to fail because the fedora-tellio.desktop file doesn't
> exist.

  As you can guess, I had that. I hesitated because the desktop files is 
included in the project. I will revert that to vendor fedora.

> 2. INSTALL NEWS README can probibly all be dropped. INSTALL is the generic
> auto* install document, NEWS and README are both of size 0.

  You are right. Done.
 
> 3. The Summary is a bit generic:
> Summary:        collection manager
> Perhaps at least add that it's a KDE based collection manager?

  Honestly I don't like to to label a program based on the framework used. 
That can be seen from its dependencies. This is build with kde and not needed 
or used by kde. It is interesting even if kde is not used... although it will 
require the dependencies to be satisfied.

> 4. rpmlint says:
> W: tellico summary-not-capitalized collection manager
> So, it should at least be "A KDE collection manager"

  I changed that to "A collection manager". If you insist I can add there the 
KDE word though, I am not dogmatic about it. :-)

Spec URL: http://www.fc.up.pt/pessoas/jamatos/fedora-extras/tellico.spec
SRPM URL: 
http://www.fc.up.pt/pessoas/jamatos/fedora-extras/tellico-1.2-2.src.rpm



Comment 3 Kevin Fenzi 2006-09-05 17:57:18 UTC
All the changes look good. I don't feel strongly about having KDE in the 
Summary. I am using the package under Xfce, so it works fine here. 

It's worth noting that due to bug: 
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=202944
I had to install 28 -devel packages to install this. Thats not this 
packages fault however. ;) 

Some new rpmlint errors (or perhaps ones I missed the first time): 

E: tellico binary-or-shlib-defines-rpath /usr/bin/tellico ['/usr/lib', '/usr/
lib/qt-3.3/lib']

Suggestion: perhaps add '--disable-rpath' to the configure line?

W: tellico dangling-symlink /usr/share/doc/HTML/en/tellico/common /usr/share/
doc/HTML/en/common
W: tellico symlink-should-be-relative /usr/share/doc/HTML/en/tellico/common /
usr/share/doc/HTML/en/common
W: tellico dangling-symlink /usr/share/doc/HTML/fr/tellico/common /usr/share/
doc/HTML/fr/common
W: tellico symlink-should-be-relative /usr/share/doc/HTML/fr/tellico/common /
usr/share/doc/HTML/fr/common

Suggestion: change to relative symlinks?

Comment 4 Kevin Fenzi 2006-09-17 23:27:52 UTC
Any further progress on this submission? 
Did you see the suggestions in comment #3?


Comment 5 José Matos 2006-09-19 19:38:38 UTC
(In reply to comment #4)
> Any further progress on this submission? 

  I am sorry. I have been busy with the deadline on FC6 rebuild.

> Did you see the suggestions in comment #3?

  I am commenting them bellow.

(In reply to comment #3)
> All the changes look good. I don't feel strongly about having KDE in the 
> Summary. I am using the package under Xfce, so it works fine here. 

  Good. :-)

> It's worth noting that due to bug: 
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=202944
> I had to install 28 -devel packages to install this. Thats not this 
> packages fault however. ;) 

  Good.

> Some new rpmlint errors (or perhaps ones I missed the first time): 
> 
> E: tellico binary-or-shlib-defines-rpath /usr/bin/tellico 
['/usr/lib', '/usr/
> lib/qt-3.3/lib']
> 
> Suggestion: perhaps add '--disable-rpath' to the configure line?

  Done.

> W: tellico 
dangling-symlink /usr/share/doc/HTML/en/tellico/common /usr/share/
> doc/HTML/en/common
> W: tellico 
symlink-should-be-relative /usr/share/doc/HTML/en/tellico/common /
> usr/share/doc/HTML/en/common
> W: tellico 
dangling-symlink /usr/share/doc/HTML/fr/tellico/common /usr/share/
> doc/HTML/fr/common
> W: tellico 
symlink-should-be-relative /usr/share/doc/HTML/fr/tellico/common /
> usr/share/doc/HTML/fr/common
> 
> Suggestion: change to relative symlinks?

  What is the problem with absolute symlinks? I am just asking. In this 
particular case I get:

$ rpm -qf /usr/share/doc/HTML/
fedora-release-5-5
kdelibs-3.5.4-5.0.fc5.kde

  So the directory is always owned and it is pulled from the requirements.

The package with only the symlink issue, and a new upstream release is here:
Spec URL: http://www.fc.up.pt/pessoas/jamatos/fedora-extras/tellico.spec
SRPM URL: 
http://www.fc.up.pt/pessoas/jamatos/fedora-extras/tellico-1.2.2-1.src.rpm


Comment 6 Kevin Fenzi 2006-09-19 21:45:59 UTC
>  What is the problem with absolute symlinks? I am just asking.

Well, rpmlint says: 

symlink-should-be-relative :
Absolute symlinks are problematic eg. when working with chroot environments.

While it would be good to sometime fix this, I don't guess it's a blocker, as 
many other kde packages do the same thing and I don't know how many people 
would use the package in a chroot. You might investigate if it would be easy to 
fix.

Everything else looks good, so this package is APPROVED. 
Don't forget to close this bug with NEXTRELEASE once the package has been 
imported and built. 

Comment 7 José Matos 2006-09-19 23:40:40 UTC
Imported and built. Thanks for the review.


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