---------- Mensaje reenviado ---------- De: Iker Salmón San Millán <sha...@esdebian.org> Fecha: 1 de noviembre de 2010 13:35 Asunto: Re: RFS: wicd-client-kde Para: Didier 'OdyX' Raboud <did...@raboud.com>
2010/11/1 Andreas Ronnquist <gus...@gusnan.se> > On Mon, 1 Nov 2010 11:30:07 +0100 > Iker Salmón San Millán <sha...@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 <gus...@gusnan.se> > > Thanks for the help 2010/11/1 Didier 'OdyX' Raboud <did...@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.
