Bug 688886 - (kflickr) Review Request: kflickr - Standalone Flickr Uploader
Review Request: kflickr - Standalone Flickr Uploader
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Extras Quality Assurance
: Reopened
Depends On:
Blocks: kde-reviews
  Show dependency treegraph
 
Reported: 2011-03-18 07:53 EDT by Jan Klepek
Modified: 2012-01-07 11:28 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-01-07 11:28:08 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rdieter: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jan Klepek 2011-03-18 07:53:29 EDT
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 11:34:14 EDT
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 10:51:40 EDT
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 16:27:28 EDT
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-27 20:32:46 EDT
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-27 20:35:17 EDT
(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 17:55:51 EDT
What's needed for the thumbnailing is probably a plugin, not a hard, directly linked dependency.
Comment 8 Rex Dieter 2011-05-20 10:16:23 EDT
I can review.
Comment 9 Rex Dieter 2011-07-14 12:59:56 EDT
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 14:23:10 EDT
Looks good, thanks!   APPROVED.
Comment 12 Jan Klepek 2011-09-02 09:12:44 EDT
New Package CVS Request
=======================
Package Name: kflickr
Short Description: Standalone Flickr Uploader
Owners: hpejakle
Branches: F-14 F-15 F-16
InitialCC:
Comment 13 Jon Ciesla 2011-09-02 09:21:18 EDT
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 08:56:23 EDT
Package Change Request
======================
Package Name: kflickr
New Branches: F-15 F-16
Owners: hpejakle
InitialCC:
Comment 15 Jon Ciesla 2011-09-06 09:19:17 EDT
Git done (by process-git-requests).
Comment 16 Rex Dieter 2012-01-07 10:55:43 EST
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 11:28:08 EST
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.