On Sun, Nov 1, 2015 at 9:01 PM, Vern Sun wrote: > I am looking for a sponsor for "ranger":
I don't intend to sponsor this but here is a review: Why is this patch marked as not needing forwarding? It fixes a bug in the upstream code and so it should be fixed upstream. 0004-closes-772351-thanks-Raphael-Geissert.patch According to debian/changelog this patch has been merged upstream, you can remove it from debian/patches and remove the commented line about it in debian/patches/series. 0003-honour-TMPDIR-environment-variable.patch I expect this patch should not have been sent upstream as sensible-utils is probably not installed very often outside of Debian systems. 0002-make-sensible-decisions-on-which-pager-and-editor.patch I would suggest running `wrap-and-sort -sa` to wrap and sort the debian/control file and other files. The Priority should probably be optional instead of extra. HACKING.md doesn't need to be installed in the binary package (see debian/docs) as it is a file for people who want to modify the source code. You might want to include some upstream metadata: https://wiki.debian.org/UpstreamMetadata doc/*.1 are generated files that should be built from source only. Please ask upstream to generate them at build time and get them removed from the upstream VCS and tarballs. If they refuse you should remove them in both `debian/rules clean` and `debian/rules build` and generate them during `debian/rules build`. Upstream has two CHANGELOG files that are identical apart from the formatting and the 2015-10-04: version 1.7.2 entry, they probably should delete CHANGELOG.md or decide which one they are going to use. Generally the AUTHORS file only contains authors and license information is placed in COPYING, you might want to suggest that to upstream. ranger/__init__.py and ranger/core/main.py probably have arbitrary file overwrite vulnerabilities. Upstream should use either the user's home directory or tempfile.TemporaryFile or tempfile.NamedTemporaryFile instead of tempfile.gettempdir(). examples/plugin_ipc.py is creating FIFOs in /tmp, I would suggest that should be fixed to use $XDG_RUNTIME_DIR or somewhere under the user's home directory. Upstream might like to rewrite the UI parts using urwid, which is a high-level python console widget library. I imagine it would be more pleasant to use than raw ncurses. http://urwid.org/ ./ranger/data/scope.sh uses env to find sh but it is always at /bin/sh so upstream doesn't need to use env. The upstream signing key uses a SHA-1 self-signature, you might want to educate them about OpenPGP best practices, push out a key with a SHA-2 self-signature and update the signing key in the debian/upstream/signing-key.asc using export-minimal: https://help.riseup.net/en/security/message-security/openpgp/best-practices https://wiki.debian.org/debian/watch#Cryptographic_signature_verification Also, lintian indicates that debian/watch is missing the things needed to verify the upstream tarball using debian/upstream/signing-key.asc . When you are contacting upstream, please mention the upstream guide: https://wiki.debian.org/UpstreamGuide Automatic checks: lintian P: ranger source: no-dep5-copyright P: ranger source: debian-watch-may-check-gpg-signature build dpkg-gencontrol: warning: package ranger: unused substitution variable ${python:Versions} check-all-the-things $ cme check dpkg ... Warning in 'control source Build-Depends:1' value 'dh-python (>= 1)': unnecessary versioned dependency: dh-python >= 1. Debian has wheezy-backports -> 1.20140511-1~bpo70+1; jessie-kfreebsd -> 1.20141111-2; jessie -> 1.20141111-2; stretch -> 2.20150826; sid -> 2.20150826; Warning in 'control source Build-Depends:3' value 'ncurses-bin (>= 5.7+20100313-5)': unnecessary versioned dependency: ncurses-bin >= 5.7+20100313-5. Debian has squeeze -> 5.7+20100313-5; wheezy -> 5.9-10; jessie-kfreebsd -> 5.9+20140913-1+b1; jessie -> 5.9+20140913-1+b1; stretch -> 6.0+20150810-1; sid -> 6.0+20151024-1; File debian/copyright line 1 has a syntax error: DpkgSyntax error: Invalid line (missing ':' ?) : This is Debian's prepackaged version of Ranger, A file manager with an ncurses $ codespell --quiet-level=3 ./CHANGELOG:117: noticable ==> noticeable ./CHANGELOG:125: certian ==> certain ./CHANGELOG.md:103: noticable ==> noticeable ./CHANGELOG.md:111: certian ==> certain ./ranger/gui/displayable.py:35: reccomended ==> recommended ./ranger/gui/curses_shortcuts.py:18: faciliate ==> facilitate ./ranger/config/rc.conf:139: Noticable ==> Noticeable ./ranger/core/main.py:234: faciliate ==> facilitate ./ranger/ext/keybinding_parser.py:74: cant ==> can't ./examples/rc_emacs.conf:140: Noticable ==> Noticeable ./doc/colorschemes.txt:91: maintainance ==> maintenance ./doc/config/rc.conf:139: Noticable ==> Noticeable $ find -type f -iname '*.asc' -exec cat {} + | hot dearmor | hokey lint ... Self-sig hash algorithms: [SHA1] $ pep8 --ignore W191 . <lots of warnings> $ pyflakes . <lots of warnings> $ find -type f \( -iname '*.sh' -o -iname '*.bash' -o -iname '*.zsh' \) -exec shellcheck {} + <lots of warnings> -- bye, pabs https://wiki.debian.org/PaulWise
