Re: RFS: gtk2-engines-cleanice: adopted, ITA #553506
Dear Rogério,
Thanks for your detailed review! My comments follow below:
On Sun, Mar 28, 2010 at 06:41:18AM -0300, Rogério Brito wrote:
> Dear Stanislav:
>
> On Mar 27 2010, Stanislav Maslovski wrote:
> > I have adopted gtk2-engines-cleanice (ITA bug #553506).
> > The package source format has been changed to 3.0 (quilt).
> > The orig.tar.gz has not been changed.
>
> That is quite nice. I am not a DD and I just did a quick review here,
> with the following results:
>
> * the source is lintian clean, even with pedantic and experimental. Good.
> * the md5sum of the provided tarball matches the one from upstream. Good.
> * the license in the sources is GPL-2, not GPL-2+ as is stated in
> debian/copyright.
Good eyes! I did not notice it was incorrect and, respectively, did
not touch that part. I will correct this.
> * the license in the sources list the old address of the FSF.
Hm, what do you suggest? Provide a patch?
> * the indication of the full text of the GPL in debian/copyright is
> unversioned. Since the unversioned path links to GPL-3, you have to
> use the versioned file.
Agreed.
> * the long description of the package lists the author: is this
> necessary?
> * the short description mentions "CleanIce themes", in plural, but the
> long description mention "a clean ice theme" right at the beginning.
> Then, the long description says that the package provides 3 styles.
> That should, perhaps, be modified?
I will look at this.
> * the debian/docs file installs the README file, but it is very short
> and contains instructions to compile from source and how to use the
> theme. I guess that you could remove that and include just the
> information on how to use it in the README.Debian file.
Okay, I was also thinking about removing that README.
> * perhaps you would want to add the package lxappearance as a
> Recommends: or Suggests: ?
Hm, I think it should be the other way around.
> * similarly to the README file, the AUTHORS file has its entire contents
> in debian/copyright. Is this duplication necessary?
Yep, my feeling was the same, I just kept it because it was in the
original package. I will remove it.
> * some of the files in debian/rules have some trailing whitespace, but
> this is just nitpicking...
I will look at this also.
Many thanks for reviewing!
--
Stanislav
Reply to: