Page MenuHomePhabricator

Don't bundle PEP8
ClosedPublic

Authored by joshuaspence on Jun 9 2014, 3:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Feb 28, 12:21 PM
Unknown Object (File)
Feb 4 2024, 4:00 AM
Unknown Object (File)
Dec 27 2023, 2:00 PM
Unknown Object (File)
Dec 22 2023, 4:03 AM
Unknown Object (File)
Dec 19 2023, 12:01 AM
Unknown Object (File)
Dec 13 2023, 7:14 PM
Unknown Object (File)
Nov 20 2023, 6:41 AM
Unknown Object (File)
Nov 5 2023, 7:28 AM

Details

Summary

Currently, we bundle a specific version of pep8 with Arcanist. Whilst maybe this made sense historically, as pointed out by @epriestley in D9412#6, there is no longer much point in doing so.

Test Plan

N/A

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

joshuaspence retitled this revision from to Don't bundle `pep8.py`..
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
joshuaspence added a subscriber: epriestley.
joshuaspence retitled this revision from Don't bundle `pep8.py`. to [LATER] Don't bundle `pep8.py`..Jun 11 2014, 1:14 AM
joshuaspence edited edge metadata.
joshuaspence retitled this revision from [LATER] Don't bundle `pep8.py`. to [LATER] Don't bundle `pep8.py`.Sep 22 2014, 2:24 PM
joshuaspence retitled this revision from [LATER] Don't bundle `pep8.py` to Don't bundle `pep8.py`.Dec 30 2014, 11:36 PM

FWIW, a colleague of mine was confused by the existing behavior. Specifically, it was unclear why arc lint was looking for python2.6 (presumably, he had python2.7 installed locally). The "fix" was to pip install pep8.

joshuaspence retitled this revision from Don't bundle `pep8.py` to Don't bundle PEP8.Feb 8 2015, 11:19 AM
talshiri added inline comments.
src/lint/linter/ArcanistPEP8Linter.php
43–44

Feels like this should not be deleted. Instead, binaryExists() should potentially be used to figure out if pep8 is even installed, and if not raise an exception.

This would be more like how ArcanistCSharpLinter works

44

jshint is the default?

epriestley edited edge metadata.

jshint default is definitely wrong.

This revision now requires changes to proceed.May 19 2015, 1:27 PM
joshuaspence added inline comments.
src/lint/linter/ArcanistPEP8Linter.php
43–44

This should already happen in ArcanistExternalLinter.

joshuaspence edited edge metadata.
joshuaspence marked an inline comment as done.

Fix bad copy-paste

epriestley edited edge metadata.
This revision is now accepted and ready to land.May 19 2015, 2:15 PM
This revision was automatically updated to reflect the committed changes.