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

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.


> * 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!


Reply to: