El día 19 de abril de 2012 04:06, Ryan Niebur <r...@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 -- To UNSUBSCRIBE, email to debian-wnpp-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/caji_fcjn+kq-ssyxsk3x0j2uohyimrqymqkodwqzq1yghem...@mail.gmail.com
