Re: RFS: ranger/1.7.2-1 (new upstream release)
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
Reply to: