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

Bug#663577: irssi-scripts-20120326 new release



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

-- 



Reply to: