On 10:48 Mon 30 Jul , Jakub Wilk wrote: > This is only a very rudimentary review. I don't have time to review > this properly for the time being. Anybody else is welcome to do it > for me. :)
Thanks.. Wonder how many more silly stuffs show up on actual review ;-) > > * Vasudev Kamath <kamathvasu...@gmail.com>, 2012-07-29, 22:27: > > dget -x > > http://mentors.debian.net/debian/pool/main/s/suckless-tools/suckless-tools_39-1.dsc > > > > More information about hello can be obtained from http://www.example.com. > > > > Changes since the last upload: > > > >suckless-tools (39-1) unstable; urgency=low > > It doesn't look like it's suitable for wheezy, so please make it > s/unstable/experimental/. Done! When it should be moved to unstable? After wheezy release? It does contains some new version of tools so asking. > > > [ Michael Stummvoll ] > > * New Maintainer (Closes: #647090) > > [0] > > > [ Vasudev Kamath ] > > * Imported new version of slock (Closes: #667796) > > This fixes a security issue, so please mention CVE number in the > changelog. Done > > > + Added myself as maintainer and Michael Stummvoll as Uploader > > I'd merge this item with [0]. Done > > > + Added dependency on dpkg-dev >= 1.16.1.1 > > It'd nice to mention why it's needed. Did it :-) > > >+This package contains a set of tools from suckless community as > >+single package. To build the package you need to create source > >+tarballs of individual tool component involved. This can be done > >+by running following command from suckless-tools folder > >+ > >+ fakeroot debian/rules get-orig-source > > Why fakeroot? Well by habit wrote it :-).. Now fixed > > >+Forwarded: <no|not-needed|url proving that it has been forwarded> > > Please choose one. :) Ouch.. I will end up doing one or other copy paste error. Fixed > > >+-$ $(tabbed -d >/tmp/tabbed.xid); urxvt -embed $(</tmp/tabbed.xid); > >++$ $(tabbed \-d >/tmp/tabbed.xid); urxvt \-embed $(</tmp/tabbed.xid); > > If you're fixing this, please also fix the security hole (insecure > use of temporary files). Done too > > >+override_dh_installdocs: > >+ dh_installdocs > >+ for TOOL in $(TOOLS); \ > >+ do \ > >+ cp $${TOOL}/README > >$(D)/usr/share/doc/suckless-tools/README.$${TOOL}; \ > >+ done > > This for loop needs a "set -e"; see Policy ยง4.6. I see other parts > of debian/rules has the same problem. As discussed in IRC the SHELL := sh -e on top of rule file should handle this. > > >+ @cd /tmp > >+ @tar -cvf - suckless-tools_$(CURRENT_VERSION) 2> /dev/null | gzip -9 > > >../suckless-tools_$(CURRENT_VERSION).orig.tar.gz > >+ @rm -rf /tmp/suckless-tools_$(CURRENT_VERSION) > > This creates temporary files insecurely. Fixed. Instead of pushing new package to mentors I've pushed my changes to collab-maint repository [1] Hope that is fine with you if not let me know [1] git.debian.org:/git/collab-maint/suckless-tools.git Thanks for the review. -- Vasudev Kamath http://copyninja.info Connect on ~friendica: copyninja@{frndk.de | vasudev.homelinux.net} IRC nick: copyninja | vasudev {irc.oftc.net | irc.freenode.net} GPG Key: C517 C25D E408 759D 98A4 C96B 6C8F 74AE 8770 0B7E
signature.asc
Description: Digital signature
