This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8619

tests: skip pyflakes for mercurial/thirdparty/
ClosedPublic

Authored by av6 on Jun 6 2020, 7:31 AM.

Download Raw Diff

Details

Reviewers

pulkit

Group Reviewers

hg-reviewers

Commits

rHGa4438263b228: tests: skip pyflakes for mercurial/thirdparty/

Summary

The current version of pyflakes (2.2.0) correctly detects one issue:

mercurial/thirdparty/selectors2.py:335:40 '...'.format(...) has unused arguments at position(s): 1

But we're not interested in fixing lint errors in third-party code, so we need
to exclude at least selectors2.py. And in the discussion for this patch it was
decided to just skip the entire thirdparty directory.

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

av6 created this revision.Jun 6 2020, 7:31 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptJun 6 2020, 7:31 AM

Herald added a subscriber: mercurial-patches. · View Herald Transcript

mjacob added a subscriber: mjacob.Jun 6 2020, 8:47 AM

mjacob added inline comments.

tests/test-check-pyflakes.t
19	Wouldn’t it make sense to skip everything in `mercurial/thirdparty`?

av6 added inline comments.Jun 6 2020, 9:09 AM

tests/test-check-pyflakes.t
19	That's what I'd think too, but maybe there's a reason why only certain directories/files from thirdparty/ are listed here. I'll let someone who knows more about the thirdparty directory make the call.

marmoute added subscribers: indygreg, marmoute.Jun 8 2020, 6:16 AM

marmoute added inline comments.

tests/test-check-pyflakes.t
19	cc @indygreg

pulkit added a subscriber: pulkit.Jun 9 2020, 5:14 AM

pulkit added inline comments.

tests/test-check-pyflakes.t
19	I think we can just skip everything in `mercurial/thirdparty`.

av6 retitled this revision from tests: skip pyflakes for selectors2.py to tests: skip pyflakes for mercurial/thirdparty/.Jun 9 2020, 8:34 AM

av6 edited the summary of this revision. (Show Details)

av6 updated this revision to Diff 21587.

pulkit accepted this revision.Jun 11 2020, 9:06 AM

This revision is now accepted and ready to land.Jun 11 2020, 9:06 AM

av6 added a commit: rHGa4438263b228: tests: skip pyflakes for mercurial/thirdparty/.Jun 11 2020, 9:39 AM

Closed by commit rHGa4438263b228: tests: skip pyflakes for mercurial/thirdparty/ (authored by av6). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			tests/test-check-pyflakes.t (4 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	21572		Jun 6 2020, 7:31 AM	★	★
Diff 2	21587		Jun 9 2020, 8:34 AM	★	★
Diff 3	21606	rHGa4438263b228dd3e2983d59095c6180b1411f0e8	Jun 6 2020, 7:19 AM	★	★

Status	Author	Revision
Closed	av6	D8619 tests: skip pyflakes for mercurial/thirdparty/
Closed	av6	D8618 tests: adjust to the new format in pyflakes output
Closed	av6	D8617 tests: consistently use pyflakes as a Python module

Diff 21606

tests/test-check-pyflakes.t

	$ $PYTHON -m pyflakes test.py 2>/dev/null \| "$TESTDIR/filterpyflakes.py"			$ $PYTHON -m pyflakes test.py 2>/dev/null \| "$TESTDIR/filterpyflakes.py"
	test.py:1:* undefined name 'undefinedname' (glob)			test.py:1:* undefined name 'undefinedname' (glob)

	$ cd "`dirname "$TESTDIR"`"			$ cd "`dirname "$TESTDIR"`"

	$ testrepohg locate 'set:*.py or grep("^#!.python")' \			$ testrepohg locate 'set:*.py or grep("^#!.python")' \
	> -X hgext/fsmonitor/pywatchman \			> -X hgext/fsmonitor/pywatchman \
	> -X mercurial/pycompat.py -X contrib/python-zstandard \			> -X mercurial/pycompat.py -X contrib/python-zstandard \
	> -X mercurial/thirdparty/cbor \			> -X mercurial/thirdparty \
				mjacobUnsubmitted Not Done Wouldn’t it make sense to skip everything in `mercurial/thirdparty`? mjacob: Wouldn’t it make sense to skip everything in `mercurial/thirdparty`?
				av6AuthorUnsubmitted Done That's what I'd think too, but maybe there's a reason why only certain directories/files from thirdparty/ are listed here. I'll let someone who knows more about the thirdparty directory make the call. av6: That's what I'd think too, but maybe there's a reason why only certain directories/files from…
				marmouteUnsubmitted Not Done cc @indygreg marmoute: cc @indygreg
				pulkitUnsubmitted Not Done I think we can just skip everything in `mercurial/thirdparty`. pulkit: I think we can just skip everything in `mercurial/thirdparty`.
	> -X mercurial/thirdparty/concurrent \
	> -X mercurial/thirdparty/zope \
	> 2>/dev/null \			> 2>/dev/null \
	> \| xargs $PYTHON -m pyflakes 2>/dev/null \| "$TESTDIR/filterpyflakes.py"			> \| xargs $PYTHON -m pyflakes 2>/dev/null \| "$TESTDIR/filterpyflakes.py"
	contrib/perf.py:: undefined name 'xrange' (glob) (?)			contrib/perf.py:: undefined name 'xrange' (glob) (?)
	mercurial/hgweb/server.py:: undefined name 'reload' (glob) (?)			mercurial/hgweb/server.py:: undefined name 'reload' (glob) (?)
	mercurial/util.py:: undefined name 'file' (glob) (?)			mercurial/util.py:: undefined name 'file' (glob) (?)
	mercurial/encoding.py:: undefined name 'localstr' (glob) (?)			mercurial/encoding.py:: undefined name 'localstr' (glob) (?)