Bug 222456

Hi David,
A couple of needswork,

NEEDSWORK:
* Buildrequire is needed for gettext (though it builds without it)
* rpmlint complaints (on the binary)
  W: bibletime dangling-relative-symlink                                       
                                                                       
/usr/share/doc/HTML/en/bibletime/handbook/common ../../common
  W: bibletime devel-file-in-non-devel-package /usr/include/bibletimeinterface.h
  W: bibletime dangling-relative-symlink
/usr/share/doc/HTML/en/bibletime/howto/common ../../common
 The 1st and the 3rd warnings are being fixed upstream, we can wait on them for
those, the second one I believe can be ignored
 There is also a load of warnings on the desktop file installation, the attached
 patch fixes it. However to use the patch, you need to remove the add-category
and remove-key options from desktop-file-install call in the spec file. The
add-category X-Fedora had been judged to be unnecesary and the rest are taking
care of in the patch.

Other issues (in the spec):
* make bibletime.desktop is not necessary (doesn't do anything)in the build
section, the desktop file was created by configure already; that line can be removed
* I think you can also remove those preserve-root options ;)
* Calling /sbin/ldconfig in post and postun is not necessary, since bibletime
doesn't ship any library
* There is no need for the 2 lines after make install, the --dir option in the
desktop-file-install takes care of that. (By the way, i don't think it ought to
be hidden under kde subdir in %{_datadir}/applications/, bibletime can be useful
for folks who generally prefer other DE too). 
* You listed %{_datadir}/apps/bibletime/bibletimeui.rc and
%{_datadir}/apps/bibletime/tips twice, you can chmod them to 0644 in the install
section, and remove their explicit listing from the files section.
* I think its also a nice idea to use the %find_lang macro for the documentation
files as done for kio_sword.

I'm attaching a patch (to the spec) that implement fix these issues, use it as
you like.

Created attachment 145518 [details]
Desktop patch

Created attachment 145519 [details]
Patch to the bibletime.spec

New versions:

Spec URL: http://david.dw-perspective.org.uk/tmp/bibletime.spec
SRPM URL: http://david.dw-perspective.org.uk/tmp/bibletime-1.6.2-2.src.rpm

Thanks for the review. I adopted all your suggestions.

With the find_lang... I think again this is probably a no-op, as I believe 
that the i18n stuff is in a separate tarball and not in the main BT 
distribution. I was thinking of adding this at a later stage once I've got it 
into Extras.

Almost there - I hope!

(In reply to comment #4)

> With the find_lang... I think again this is probably a no-op, as I believe 
> that the i18n stuff is in a separate tarball and not in the main BT 
> distribution. 
Now thinking about it again, I believe you're right. Considering that the review
guildelines says locale files must not be explicitly listed, and since it
doesn't hurt, I think it can stay.
 
> Almost there - I hope!
Yes.

GOOD:
* rmplint silent on mock srpm
* rpmlint warning on mock built binary can be ignored (targets really exist);
(W: bibletime dangling-relative-symlink
/usr/share/doc/HTML/en/bibletime/handbook/common ../../common
W: bibletime devel-file-in-non-devel-package /usr/include/bibletimeinterface.h
- A separate -devel subpackage is not needed for this, I believe. Moreso BT
doesn't ship any library 
W: bibletime dangling-relative-symlink)
/usr/share/doc/HTML/en/bibletime/howto/common ../../common
* source tarball matches upstream's
md5sum: b2b8b624d21d397201aec742d43501e5  bibletime-1.6.2.tar.bz2
* package name meets the Package Naming Guidelines.
* Satifies the Packaging Guidelines.
* Builds in mock (x86_64 development)
* Installs and works OK (for me at least)
* License: GPL
* Spec file generally OK.

APPROVED.

It'll be nice to include the 'Changelog' with the README and License in the %doc
section before/after you eventually import it into CVS.
Also please don't forget the bibletime-i18n you promised. 

Thanks!

Imported into CVS.

I dislike changelogs in packages - useless bloat, IMO. People who want them 
know where to get them. I have 31Mb on my Fedora installation and never wanted 
any of them! (I once filed a bug against wine-core to get 8Mb of them removed 
from the package). The gcc rpm ships with 10 years of changelogs - I have no 
idea why!

Created attachment 145640 [details]
i18n patch

This is a patch for i18n. Any comments welcomed! (I plan to add it to CVS this
week).

Thanks to everyone who helped with this. Now in Extras.

(i18n being put in the devel branch as we speak - if nobody complains, I'll 
add it to FC5 + 6 in the next week).