[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Fwd: RFS: wicd-client-kde





---------- Mensaje reenviado ----------
De: Iker Salmón San Millán <shaola@esdebian.org>
Fecha: 1 de noviembre de 2010 13:35
Asunto: Re: RFS: wicd-client-kde
Para: Didier 'OdyX' Raboud <didier@raboud.com>




2010/11/1 Andreas Ronnquist <gusnan@gusnan.se>
On Mon, 1 Nov 2010 11:30:07 +0100
Iker Salmón San Millán <shaola@esdebian.org> wrote:


Hi!

I am not a DD, so I have no possiblity to upload any package, and I am
not familiar with KDE stuff - but anyway -

Trying to build (pdebuild) this package I do get a warning for lacking
cmake_minimum_required, which probably isn't too serious:

---- 8< ----
CMake Warning (dev) in CMakeLists.txt:
 No cmake_minimum_required command is present.  A line of code such as

   cmake_minimum_required(VERSION 2.8)

 should be added at the top of the file.  The version specified may be
lower if you wish to support older CMake versions for this project.
For more information run "cmake --help-policy CMP0000".
This warning is for project developers.  Use -Wno-dev to suppress it.
---- 8< ----


I didn't specify the version because the version already aviable in repositories is 2.8.2, because this package is for kde4 i tought i did not need to do it, but you are rigth, i'll fix it
What is worse, I get this too:

---- 8< ----
CMake Error at /usr/share/cmake-2.8/Modules/FindKDE4.cmake:58 (MESSAGE):
 ERROR: Could not find KDE4 kde4-config
---- 8< ----

Have you forgotten som build-depends on some KDE development package?

You are rigth, i've made so many tries that in this last one i forgot to add de dependencie, actually is kdebase-workspace-dev (this ones pulls up other dependencies needed

I am very far from an expert on packaging (I am also in the process of
trying to get a package into Debian), but as I understand reviewing of
others packages is encouraged.

best regards
--
Andreas Rönnquist <gusnan@gusnan.se>

Thanks for the help

2010/11/1 Didier 'OdyX' Raboud <didier@raboud.com>


Hi Iker, and thanks for packaging this software!

Preliminary notice: I am not a DD, so I will not be able to upload your
package. However, I think that my review can be of interest.

> The package appears to be lintian clean.

Maybe, but it doesn't build in a clean chroot: you miss at least kdelibs-bin
(so using kdelibs5-dev as Build-Dependency would be good [and works]). Did
you build it in a clean chroot (with pbuilder or sbuild e.g.) before
uploading it to mentors.d.n ?

Once I fixed that, I still had one lintian warning:

already responsed that, i forgot to add kdebase-workspace-dev as build dependencie
W: wicd-client-kde: binary-without-manpage usr/bin/wicd-client-kde

which should really be fixed (see help2man for example).

Terrible mistake, I tried man wicd-gtk to see how it was (the funcionality is the same in both front-ends and i did not find de man page.  I forgot that I uninstalled the gtk-client.  I just tough that if wicd-gtk didn't have man page this one would'nt need one. 
Silly me...

Anyway, working on that also



Other things I noticed (randomly ordered)

* Your debian/copyright is unusual as it doesn't quote the GPL license.

 I tried to do the rigth way, i'll have to see more copyrigth files to see what i've done bad. 

* You have a debian/patches subdirectory that contains direct changes to the
source: if you want to apply patches, you should really prepare real patches
(quilt add; quilt edit; …).


Yup, in this case i guess is better to correct the spelling things before dh_make.  I'll also contact with Anthony to fix that in upstream.
 
* You are mis-using the Vcs-* fields in debian/control: those are not meant
to point the the "upstream" VCS, but to the packaging VCS.

Thanks, i did not notice that
 
* This package would perfectly fit in the "KDE-Extras" team; did you
consider putting it under this umbrella ?

done
 
* There is a typo in the description: "qt" should be "Qt". And if it is
really a "KDE" client, it should be labelled as such.
 
* Your changelog contains two useless informations: the fact that it's your
first package and the two spelling errors you are fixing (without proper
patching, see above). For a first release, there is usually no need to
explain what you have done (a matter of taste). So if you want to mention
the spelling errors patch, you can, but you should mention that it is done
through patching.

thanks again
* The packages ships the network-wireless-*.png icons, that are also in knm-
runtime. So you would either need to conflict with knm-runtime or convince
their maintainers to ship said icons in a separate package that you could
use too. By the way, this shows that the icons are probably duplicated from
knm-runtime and this sucks (code/data duplication is never good). By the
way, the oxygen-icon-theme package ships network-wireless-connected-*.png
icons which could be used instead. Furthermore, those icons are shipped in
the orig tarball as *.png, which is certainly not the "preferred form of
modification".


Ok, a think this is the thing that is going to give me more work.  I'll see wich is the better way to fix it
So far so good, you have already enough to do. :-) Looking forward for your
fixed package !

Cheers, OdyX




Thank you both for your review.  Working on int

Cheers,  Iker

P.S.  Sorry Odyx, i sent the mail to you instead of debian mentors.

Reply to: