Bug 235825

Sorry for the inversion in the links :)

QT was not sourced
The icon didn't appears under gnome nor xfce
It seems a libs package was needed

New version is available : 
Spec URL: http://odysseus.x-tnd.be/fedora/kftpgrabber/kftpgrabber.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/kftpgrabber/kftpgrabber-0.8.0-2.src.rpm

unable to download SRPM

Fixed (my apache was down...).

Thank you for poiting me out.

the -devel package is missing to include 'Requires: pkgconfig'.

oops my bad ignore above comment

Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and for RPMs.
+ source files match upstream.
dbbbca5cd4303db886a2d8dac39dd98c  kftpgrabber-0.8.0.tar.bz2
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text COPYING is included in package.
+ %doc is small so no need of -doc subpackage.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no static libraries.
+ no .pc files are present.
+ -devel subpackage exists.
+ no .la files.
+ translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Desktop file installed correctly.
+ scriptlets are used.
+ kftpgrabber Provides: kftpimportplugin_filezilla3.so kftpimportplugin_gftp.so
kftpimportplugin_kftp.so kftpimportplugin_ncftp.so
+ kftpgrabber-libs Provides: libkftpinterfaces.so.0
+ GUI app.
APPROVED.

Several comments:

* Are there any benefit for splitting -libs subpackage?
  Should this package support multilib installation, for example?
  If so, please explain why.

* Sourcing /etc/profile.d/qt.sh should not be needed now.

* Usually "INSTALL" document is needed for people who want to
  install this package by themselves and should not be needed for
  people using rpm system.

(In reply to comment #7)
> Several comments:
> 
> * Are there any benefit for splitting -libs subpackage?
   Do we always need to ask this question to submitter?

>   Should this package support multilib installation, for example?
>   If so, please explain why.
> 
> * Sourcing /etc/profile.d/qt.sh should not be needed now.
   ohh I have less experience on KDE package reviewing. Is that somewhere
mentioned (may be I missed to read that)?
> 
> * Usually "INSTALL" document is needed for people who want to
>   install this package by themselves and should not be needed for
>   people using rpm system.
    Why should we so bother about INSTALL file installation?

mtasaka,
thanks for pointing out some missing things.
removing fedora-review+ flag for now.

(In reply to comment #8)
> (In reply to comment #7)
> > Several comments:
> > 
> > * Are there any benefit for splitting -libs subpackage?
>    Do we always need to ask this question to submitter?
Splitting -libs package usually means that there are
_other_ software which uses only this -libs package and does
not require the "main" package of this software.

We should avoid unnecessary splitting of subpackage, which
causes only confusion.

> > * Usually "INSTALL" document is needed for people who want to
> >   install this package by themselves and should not be needed for
> >   people using rpm system.
>     Why should we so bother about INSTALL file installation?

Just because "don't install unnecessary files".
For INSTALL files, I see many people including INSTALL file without
any consideration and for most cases this file is not necessary.
The other case is that people don't include AUTHORS or COPYING
files.. 

We must check what files should be installed as documentation
carefully. On several (not a few) review requests, I comment
about documentation like "This file should be included as documentation"
or "This file is not necessary"

New version is available : 
Spec URL: http://odysseus.x-tnd.be/fedora/kftpgrabber/kftpgrabber.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/kftpgrabber/kftpgrabber-0.8.1-1.src.rpm

- Updated to version 0.8.1
- I've removed the libs subpackage

As for the INSTALL file, I really don't know if it should be removed or not, so
I let it for the devel package for the moment.

Ok. So when I use "locate INSTALL" on FC6 system I got many packages using INSTALL.

Will review this on Monday.   

Ok requesting you to remove INSTALL as it really increases unnecessary files
count on system.

Done, INSTALL is removed, I'll remove it from all other packages I will build
from now :)

Here is the last version package :
Spec URL: http://odysseus.x-tnd.be/fedora/kftpgrabber/kftpgrabber.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/kftpgrabber/kftpgrabber-0.8.1-2.src.rpm

Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and for RPMs.
+ source files match upstream.
56610fd3e2e7f092b7d8eed10d3e5d36  kftpgrabber-0.8.1.tar.bz2
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text COPYING is included in package.
+ %doc is small so no need of -doc subpackage.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no static libraries.
+ no .pc files are present.
+ -devel subpackage exists.
+ no .la files.
+ translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Desktop file installed correctly.
+ scriptlets are used.
+ Provides: kftpimportplugin_filezilla3.so kftpimportplugin_gftp.so
kftpimportplugin_kftp.so kftpimportplugin_ncftp.so libkftpinterfaces.so.0
+ GUI app.
APPROVED.

New Package CVS Request
=======================
Package Name: kftpgrabber
Short Description: FTP client for K Desktop Environment
Owners: johan
Branches: FC-6
InitialCC: johan

What is the status of this bug?

The package is now on the repositories for FC6 and F7, this bug should be closed
(I don't know if I can do this, and even, how to...)

Just to change "Bug Status Change" written below.
For now I will change.

Package Change Request
======================
Package Name: kftpgrabber
New Branches: F-7

cvs done.