This is an archive of the discontinued Mercurial Phabricator instance.

Differential D15

contrib: widen "direct use of `python`" net again
ClosedPublic

Authored by durin42 on Jul 5 2017, 12:35 PM.
Tags
None
Subscribers
dsp

Details

Reviewers
krbullock
Commits
rHGf08a178adadf: contrib: widen "direct use of `python`" net again
Summary

I think I've now caught all of them.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.Jul 5 2017, 12:35 PM
dsp added a subscriber: dsp.Jul 6 2017, 4:48 AM
dsp added inline comments.
contrib/check-code.py
141

can we just make this a character class, seems easier to read?

r'[\s\'"`=]python\s(?!bindings)', ...
dsp added a comment.Jul 6 2017, 4:53 AM

Note that tests/test-demandimport.py, and tests/test-filecache.py call subprocess.call(['python',..., do we want to catch this to use $PYTHON (or anything else that is not literal, e.g. os.environ.get('PYTHON', 'python') as well? If so we might want to check for ['

durin42 updated this revision to Diff 27.Jul 6 2017, 3:21 PM
durin42 marked an inline comment as done.Jul 6 2017, 3:22 PM
durin42 added inline comments.
contrib/check-code.py
141

Nice catch.

krbullock accepted this revision.Jul 6 2017, 3:43 PM
This revision is now accepted and ready to land.Jul 6 2017, 3:43 PM
dsp changed the visibility from "All Users" to "Public (No Login Required)".Jul 6 2017, 5:40 PM
durin42 marked an inline comment as done.Jul 7 2017, 10:34 AM
Closed by commit rHGf08a178adadf: contrib: widen "direct use of `python`" net again (authored by durin42). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mcontrib/check-code.py (2 lines)

Diff 32

contrib/check-code.py

(r'if\s*!', "don't use '!' to negate exit status"), (r'if\s*!', "don't use '!' to negate exit status"),
(r'/dev/u?random', "don't use entropy, use /dev/zero"), (r'/dev/u?random', "don't use entropy, use /dev/zero"),
(r'do\s*true;\s*done', "don't use true as loop body, use sleep 0"), (r'do\s*true;\s*done', "don't use true as loop body, use sleep 0"),
(r'^( *)\t', "don't use tabs to indent"), (r'^( *)\t', "don't use tabs to indent"),
(r'sed (-e )?\'(\d+|/[^/]*/)i(?!\\\n)', (r'sed (-e )?\'(\d+|/[^/]*/)i(?!\\\n)',
"put a backslash-escaped newline after sed 'i' command"), "put a backslash-escaped newline after sed 'i' command"),
(r'^diff *-\w*[uU].*$\n(^ \$ |^$)', "prefix diff -u/-U with cmp"), (r'^diff *-\w*[uU].*$\n(^ \$ |^$)', "prefix diff -u/-U with cmp"),
(r'^\s+(if)? diff *-\w*[uU]', "prefix diff -u/-U with cmp"), (r'^\s+(if)? diff *-\w*[uU]', "prefix diff -u/-U with cmp"),
(r'\spython\s(?!bindings)', "don't use 'python', use '$PYTHON'"), (r'[\s="`\']python\s(?!bindings)', "don't use 'python', use '$PYTHON'"),
dspUnsubmitted
Done

can we just make this a character class, seems easier to read?

r'[\s\'"`=]python\s(?!bindings)', ...
dsp: can we just make this a character class, seems easier to read? ``` r'[\s\'"`=]python\s(?!
durin42AuthorUnsubmitted
Not Done

Nice catch.

durin42: Nice catch.
(r'seq ', "don't use 'seq', use $TESTDIR/seq.py"), (r'seq ', "don't use 'seq', use $TESTDIR/seq.py"),
(r'\butil\.Abort\b', "directly use error.Abort"), (r'\butil\.Abort\b', "directly use error.Abort"),
(r'\|&', "don't use |&, use 2>&1"), (r'\|&', "don't use |&, use 2>&1"),
(r'\w = +\w', "only one space after = allowed"), (r'\w = +\w', "only one space after = allowed"),
(r'\bsed\b.*[^\\]\\n', "don't use 'sed ... \\n', use a \\ and a newline"), (r'\bsed\b.*[^\\]\\n', "don't use 'sed ... \\n', use a \\ and a newline"),
(r'env.*-u', "don't use 'env -u VAR', use 'unset VAR'"), (r'env.*-u', "don't use 'env -u VAR', use 'unset VAR'"),
(r'cp.* -r ', "don't use 'cp -r', use 'cp -R'"), (r'cp.* -r ', "don't use 'cp -r', use 'cp -R'"),
(r'grep.* -[ABC] ', "don't use grep's context flags"), (r'grep.* -[ABC] ', "don't use grep's context flags"),