On Wed, Mar 18, 2015 at 6:34 AM, Harlan Lieberman-Berg wrote: > Thank you for your work on the s3cmd package. I'm not able to sponsor > your package at this time, but I've done a review for you to help fix up > a couple of nitpicks while you wait.
All good points Harlan. I also won't be sponsoring this. > the change in d/copyright from GPL-2 to GPL-2+ for the files under debian/. I think the debian/copyright issue should be a blocker for this upload. > probably with the tool they are using to create the manpage itself That tool doesn't appear to be run at build time to generate the manual page, which means that if downstream folks patch the command they won't get an updated manual page. I'd strongly suggest that the manual page be removed from the upstream git repos and tarballs and that the manual page be created at package build time. The script used to create the manual page would have to be added to the tarballs, currently it is only available in git. > That will require changing the watch file from github to sourceforge. An alternative would be to ask upstream to upload all of their tarballs and detached OpenPGP signatures to GitHub in addition to SourceForge. I'd recommend that they do that anyway so people who prefer GitHub can get them from there. A few more minor things: python-magic is dropped from the recommends and isn't in depends, was that intended? I'd suggest using debdiff or debbindiff to compare binary packages before future uploads. debian/docs has an unnecessary blank line. Usually there are blank lines between the different contributors in debian/changelog, did you use dch to prepare the changelog? The homepage change isn't mentioned in debian/changelog. I'd suggest replacing /tmp/ with ~/ in the README.md examples and the manual page. As this tool is for interacting with a remote network service, I'd recommend implementing some post-install tests using the mechanisms described in DEP-8. http://dep.debian.net/deps/dep8/ It would be good to get one or two screenshots illustrating how to use this tool. https://screenshots.debian.net/upload A build-time warning: I: dh_python2 pydist:184: Cannot find package that provides python_magic. Please add package that provides it to Build-Depends or add "python_magic python-magic-fixme" line to debian/pydist-overrides or add proper dependency to Depends by hand and ignore this info. lintian: P: s3cmd source: debian-watch-may-check-gpg-signature P: s3cmd: no-upstream-changelog X: s3cmd: duplicate-files usr/share/doc/packages/s3cmd/README.md.gz usr/share/doc/s3cmd/README.md.gz W: s3cmd: manpage-has-errors-from-man usr/share/man/man1/s3cmd.1.gz 365: warning: macro `Cache-Control'' not defined I: s3cmd: spelling-error-in-manpage usr/share/man/man1/s3cmd.1.gz overriden overridden I: s3cmd: hyphen-used-as-minus-sign usr/share/man/man1/s3cmd.1.gz:289 I: s3cmd: hyphen-used-as-minus-sign usr/share/man/man1/s3cmd.1.gz:296 I: s3cmd: hyphen-used-as-minus-sign usr/share/man/man1/s3cmd.1.gz:301 I: s3cmd: hyphen-used-as-minus-sign usr/share/man/man1/s3cmd.1.gz:304 I: s3cmd: hyphen-used-as-minus-sign usr/share/man/man1/s3cmd.1.gz:307 I: s3cmd: hyphen-used-as-minus-sign usr/share/man/man1/s3cmd.1.gz:311 $ pep8 --ignore W191 . <lots of warnings> $ pyflakes . <lots of warnings> -- bye, pabs https://wiki.debian.org/PaulWise -- To UNSUBSCRIBE, email to debian-mentors-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: https://lists.debian.org/caktje6ht8zsvffpvoupagwsjmgdsvk8nkutptw3dnkqptn2...@mail.gmail.com
