Bug 688886 (kflickr) - Review Request: kflickr - Standalone Flickr Uploader
Summary: Review Request: kflickr - Standalone Flickr Uploader
Keywords:
Status: CLOSED RAWHIDE
Alias: kflickr
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: kde-reviews
TreeView+ depends on / blocked
 
Reported: 2011-03-18 11:53 UTC by Jan Klepek
Modified: 2012-01-07 16:28 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-01-07 16:28:08 UTC
Type: ---
Embargoed:
rdieter: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Jan Klepek 2011-03-18 11:53:29 UTC
Spec URL: http://hpejakle.fedorapeople.org/packages/kflickr.spec
SRPM URL: http://hpejakle.fedorapeople.org/packages/kflickr-20100817-1.fc14.src.rpm
Description: kflickr is an easy to use photo uploader for flickr.

rpmlint:
kflickr.src: W: spelling-error %description -l en_US uploader -> unloader, uploaded, up loader
The value of this tag appears to be misspelled. Please double-check.
kflickr.src: W: spelling-error %description -l en_US flickr -> flick, flicker, flicks
The value of this tag appears to be misspelled. Please double-check.

koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2923227

Comment 1 Rex Dieter 2011-03-18 15:34:14 UTC
This combination looks out of place:

BuildRequires:  kdelibs >= 4 ...
BuildRequires:  qt3, kdelibs-devel

You want kde *4*, but also qt 3?

if it's really a kde4 app, all you really need is:

BuildRequires: kdelibs4-devel

Comment 3 Matthew Truch 2011-03-27 14:51:40 UTC
Not a full review (and I'm not taking this for now, but)...

rpmlint complains with:
kflickr.x86_64: E: explicit-lib-dependency kdebase-runtime-libs

The rpm build system will pick up library dependencies correctly and therefore the explicit lib dependency should be removed.  

Otherwise, most everything else seems good.

Comment 4 Jan Klepek 2011-03-27 20:27:28 UTC
it will not pickup this dependency automatically therefore it is explicit. I have already tried it before submitting this review request.

Comment 5 Matthew Truch 2011-03-28 00:32:46 UTC
I built the package in koji (scratch build, see link below) earlier today with the explicit dependency removed.  I can confirm that although kdebase-runtime-libs are not picked up, other kde libs are and furthermore, on a machine I have removed kdebase-runtime-libs (with yum remove kdebase-runtime-libs), installed the rpm from the scratch build, and successfully run kflickr (including registering a new account, selecting a photo, and uploading that photo to flickr).  Unless I'm missing something, kflickr does not require kdebase-runtime-libs.  

http://koji.fedoraproject.org/koji/taskinfo?taskID=2950562

Comment 6 Matthew Truch 2011-03-28 00:35:17 UTC
(In reply to comment #5)
> I built the package in koji (scratch build, see link below) earlier today with
> the explicit dependency removed.  I can confirm that although
> kdebase-runtime-libs are not picked up, other kde libs are and furthermore, on
> a machine I have removed kdebase-runtime-libs (with yum remove
> kdebase-runtime-libs), installed the rpm from the scratch build, and
> successfully run kflickr (including registering a new account, selecting a
> photo, and uploading that photo to flickr).  Unless I'm missing something,
> kflickr does not require kdebase-runtime-libs.  
> 
> http://koji.fedoraproject.org/koji/taskinfo?taskID=2950562

Wait, I see, after further testing (kdebase-runtime-libs is re-installed on the machine), the thumbnail works correctly (I assumed in the previous case that the image I had choosen was too large to by default create a thumbnail).  If this is true, it is very strange that such a dependency isn't properly picked up.

Comment 7 Kevin Kofler 2011-03-28 21:55:51 UTC
What's needed for the thumbnailing is probably a plugin, not a hard, directly linked dependency.

Comment 8 Rex Dieter 2011-05-20 14:16:23 UTC
I can review.

Comment 9 Rex Dieter 2011-07-14 16:59:56 UTC
Sorry for the delay.

1.  SHOULD: take a look at a template for advice, like,
http://fedoraproject.org/wiki/SIGs/KDE#Best_Practices
and take advantage of some %_kde4_* macros we have.

2.  MUST: use
Requires: kdebase-runtime%{?_kde4_version: >= %{_kde4_version}}
(instead of -libs)

the stuff at the end just adds some extra versioning information, to ensure the built rpm is run against at least the kde version used to build it.

3.  MUST: fix icon scriptlets, see
https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache

4.  SHOULD: drop the convoluted locale handling, and just use something like
%find_lang %{name} --with-kde

Naming OK

License ok

sources ok
a242994345de077c2a4bd07968cc217a  kflickr-20100817.tar.bz2

Comment 11 Rex Dieter 2011-09-01 18:23:10 UTC
Looks good, thanks!   APPROVED.

Comment 12 Jan Klepek 2011-09-02 13:12:44 UTC
New Package CVS Request
=======================
Package Name: kflickr
Short Description: Standalone Flickr Uploader
Owners: hpejakle
Branches: F-14 F-15 F-16
InitialCC:

Comment 13 Gwyn Ciesla 2011-09-02 13:21:18 UTC
Unretired devel and f14, please take ownership, then submit a change request
for f15 and f16 branches.  There are also el5 and el6 branches orphaned, if
you want them.

Comment 14 Jan Klepek 2011-09-06 12:56:23 UTC
Package Change Request
======================
Package Name: kflickr
New Branches: F-15 F-16
Owners: hpejakle
InitialCC:

Comment 15 Gwyn Ciesla 2011-09-06 13:19:17 UTC
Git done (by process-git-requests).

Comment 16 Rex Dieter 2012-01-07 15:55:43 UTC
Err... ping Jan, I don't see this ever got imported.  Was there a problem or something I can help with?

Comment 17 Rex Dieter 2012-01-07 16:28:08 UTC
nevermind, it was imported and built, but only for rawhide/f17.  so, we can close this.

I'd love a f16 build though... :)


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