Bug#663577: irssi-scripts-20120326 new release
2012/4/28 Ryan Niebur <ryan@debian.org>:
> Hello Daniel,
>
> Your new changes look good.
>
> Since I didn't trust myself to do a thorough reivew of the new
> copyright file (it seems like more than a person could reasonably
> handle in one sitting), I've written a script which compares it with
> the old version, which seems to have noticed some possible
> discrepancies. I will make the minor corrections for those tonight, if
> needed, before completing my review.
>
> Thanks for the patience, your contributions are appreciated.
>
> Cheers,
> Ryan
>
> On Sun, Apr 22, 2012 at 01:05:32PM -0500, Daniel Echeverry wrote:
>> El día 19 de abril de 2012 04:06, Ryan Niebur <ryan@debian.org> escribió:
>> > Hi Daniel,
>> >
>> > Thanks for your work on this, and I apologize for the delay in response.
>> >
>> > On Fri, Apr 13, 2012 at 02:39:10PM -0500, Daniel Echeverry wrote:
>> >> Hi,
>> >>
>> >> This week I've been working on packaging a new version of irssi-scripts. I
>> >> closed some bugs and updated some of the scripts. I uploaded the package to
>> >> debian mentors. [1]
>> >>
>> >> Additionally, I added a copyright.new in debian/, This file is copyright
>> >> copyright DEP5 Update to 1.0 format.
>> >>
>> >> Could you check out the new version and the new copyright file?
>> >>
>> >
>> > I've reviewed your packaging work now and overall it looks really
>> > good. Though I do wonder, have you used a git repository to track your
>> > changes? If not, we currently use a repository located here:
>> > http://anonscm.debian.org/gitweb/?p=collab-maint/irssi-scripts.git
>> >
>> > This would be preferable because it helps to make collaborating and
>> > reviewing changes easier (this would also replace the need to build
>> > and upload the source package to mentors). I think it should be okay
>> > for us to allow you to push changes to the repository there, if you
>> > want...are you a member of collab-maint on alioth?
>> >
>> > Anyways, the only major issue I've noticed is that this detail got
>> > removed from debian/rules, but the executable permission is relied on
>> > by the script:
>> > # log2ansi.pl will run outside irssi, so it needs the executable bit
>> > chmod +x $D/usr/share/irssi/scripts/log2ansi.pl
>> > So, we need to set the executable permission on this file during build.
>> >
>> > Some other more minor things I noticed are:
>> >
>> > The debian/copyright.new file looks very good, however I think the
>> > GPL-2.0 license definition should replace the text about receiving a
>> > copy of the license with a reference to /usr/share/common-licenses, as
>> > you have done with the GPL-2.0+ block. Also, the "License:GPL-2.0+" is
>> > missing a space after the colon. I think it should be fine to replace
>> > debian/copyright with your debian/copyright.new once these small
>> > improvements are made.
>> >
>> > With regards to dependencies on perl and perl-modules, this is really
>> > not necessary since the base perl, which is all that is required for
>> > most scripts, is always installed. In debian/README.Debian there is an
>> > explanation of how the dependencies are chosen (at least 3 scripts =
>> > Suggests, at least 20 scripts = Recommends), and it looks like there
>> > are actually more scripts which depend on libwww-perl than perl or
>> > perl-modules, from the list in debian/README.Debian. I'm not opposed
>> > to changing how this is done if there is value added in doing so, but
>> > if we make that choice we will need to update debian/README.Debian as
>> > well (and should possibly treat the other dependencies consistently).
>> > It might be better to just explain the way we do this to the requestor
>> > of this change instead.
>> >
>> > There were a couple patches (go.pl-multiple-networks.diff and
>> > away.pl.diff) which used to have links to bug reports instead of
>> > proper headers, and you've replaced those with DEP5 patch headers
>> > (which is great, thanks!), but I think it could be useful to still
>> > have the old bug references included within the new DEP5 information.
>> >
>> >> [1]: http://mentors.debian.net/debian/pool/main/i/irssi-scripts/
>> >> irssi-scripts_20120326.dsc
>> >>
>> >> Thank you very much!!
>> >>
>> >
>> > No, thank you!
>> >
>> > I will be able to sponsor this upload once we address the above
>> > mentioned items and I do a final review, but first I also want to hear
>> > back about if and how you want to make use of the git repository for
>> > collaboration.
>> >
>> > Cheers,
>> > Ryan
>> >
>> > -----BEGIN PGP SIGNATURE-----
>> > Version: GnuPG v1.4.10 (GNU/Linux)
>> >
>> > iEYEARECAAYFAk+P1XwACgkQMihv+PacasXwggCeOmqsvrr2lfcGsBM1VPA2LFxg
>> > hjEAn1oLlpICofxeicOIMAndpUxkVwok
>> > =MyTD
>> > -----END PGP SIGNATURE-----
>> >
>>
>> Hi,
>>
>> I have fixed all the points you mentioned, and I updated the git
>> repository on Alioth,Could you please check out again the package[1]?
>>
>> Thank You very much!!
>>
>> [1]: http://anonscm.debian.org/gitweb/?p=collab-maint/irssi-scripts.git;a=summary
>>
>>
>> --
>> Epsilon
>> http://wiki.debian.org/DanielEcheverry
>> http://www.rinconinformatico.net
>> http://www.fitnessdeportes.com
>> http://www.dragonjar.org
>> Linux user: #477840
>> Debian user
>
> --
Hi Ryan.
Ok, if you want you can send me the modifications that are required,
and I will make them right away.
Thank you very much!!
--
Epsilon
http://wiki.debian.org/DanielEcheverry
http://www.rinconinformatico.net
http://www.fitnessdeportes.com
http://www.dragonjar.org
Linux user: #477840
Debian user
Reply to: