Bug 235825 - Review Request: kftpgrabber - Graphical FTP client for KDE
Review Request: kftpgrabber - Graphical FTP client for KDE
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-10 08:17 EDT by Johan Cwiklinski
Modified: 2007-11-30 17:12 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-03 23:22:04 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Johan Cwiklinski 2007-04-10 08:17:12 EDT
Spec URL: http://odysseus.x-tnd.be/fedora/kftpgrabber/kftpgrabber-0.8.0-1.src.rpm
SRPM URL: http://odysseus.x-tnd.be/fedora/kftpgrabber/kftpgrabber.spec
Description: 
KFTPgrabber is a graphical FTP client for the K Desktop Environment.
It implements many features required for usable FTP interaction.
Comment 1 Johan Cwiklinski 2007-04-10 19:04:46 EDT
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
Comment 2 Parag AN(पराग) 2007-04-26 23:40:14 EDT
unable to download SRPM
Comment 3 Johan Cwiklinski 2007-04-27 00:49:59 EDT
Fixed (my apache was down...).

Thank you for poiting me out.
Comment 4 Parag AN(पराग) 2007-04-27 03:02:08 EDT
the -devel package is missing to include 'Requires: pkgconfig'.
Comment 5 Parag AN(पराग) 2007-04-27 03:07:46 EDT
oops my bad ignore above comment
Comment 6 Parag AN(पराग) 2007-04-27 03:15:40 EDT
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.
Comment 7 Mamoru TASAKA 2007-04-27 04:01:37 EDT
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.
Comment 8 Parag AN(पराग) 2007-04-27 05:01:07 EDT
(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?

Comment 9 Parag AN(पराग) 2007-04-27 05:02:16 EDT
mtasaka,
thanks for pointing out some missing things.
removing fedora-review+ flag for now.
Comment 10 Mamoru TASAKA 2007-04-27 05:18:37 EDT
(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"


Comment 11 Johan Cwiklinski 2007-05-12 11:19:13 EDT
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.



Comment 12 Parag AN(पराग) 2007-05-12 21:37:44 EDT
Ok. So when I use "locate INSTALL" on FC6 system I got many packages using INSTALL.

Will review this on Monday.   
Comment 13 Parag AN(पराग) 2007-05-12 22:12:22 EDT
Ok requesting you to remove INSTALL as it really increases unnecessary files
count on system.
Comment 14 Johan Cwiklinski 2007-05-13 05:38:51 EDT
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
Comment 15 Parag AN(पराग) 2007-05-13 23:47:03 EDT
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.
Comment 16 Johan Cwiklinski 2007-05-17 03:58:52 EDT
New Package CVS Request
=======================
Package Name: kftpgrabber
Short Description: FTP client for K Desktop Environment
Owners: johan@x-tnd.be
Branches: FC-6
InitialCC: johan@x-tnd.be
Comment 17 Mamoru TASAKA 2007-07-03 14:17:50 EDT
What is the status of this bug?
Comment 18 Johan Cwiklinski 2007-07-03 15:17:02 EDT
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...)
Comment 19 Mamoru TASAKA 2007-07-03 23:22:04 EDT
Just to change "Bug Status Change" written below.
For now I will change.
Comment 20 Johan Cwiklinski 2007-09-02 10:18:41 EDT
Package Change Request
======================
Package Name: kftpgrabber
New Branches: F-7
Comment 21 Kevin Fenzi 2007-09-03 14:38:04 EDT
cvs done.

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