Presubmit long line checking fails for python triple-quoted strings |
||
Issue descriptionDisabling a long line check in python requires putting a pylint disable statement in the long line. This means it cannot be used for python triple quoted strings. This is very inconvenient in particular for unittests that real-world input exceeding 80 characters, as it requires very uncomfortable string quoting gymnastics. See the diff for tombstone_utils_unittest.py in crrev.com/c/1249102 for an example. The issue seems to be that long-line testing is done manually in presubmit_canned_checks such that lines are considered individually. This means that using a pair of disable/enable directives to turn off long line testing for a region cannot be used, see no_long_lines in third_party/depot_tools/presubmit_canned_checks.py.
,
Jan 3
Trying my hand on this one now.
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/depot_tools/+/31a486419e279333146ca1e75fcffd510a863d30 commit 31a486419e279333146ca1e75fcffd510a863d30 Author: David 'Digit' Turner <digit@google.com> Date: Fri Jan 11 16:10:59 2019 Improve no-long-lines check for Python files. The previous implementation of CheckLongLines did not handle global pylint disable/enable directives properly, i.e. the difference between: # pylint: disable=line-too-long .... checks disabled for all lines here. # pylint: enable=line-too-long versus: # Check only disabled for the line below some python statements # pylint: disable=line-too-long This CL changes the implementation to support Python files properly. Note that in order to not disturb the mock-based unit-tests, a new function is introduced to be able to filter the list of affected files based on their file extension. BUG=890734 R=mattcary@chromium.org,ehmaldonado@chromium.org,dpranke@chromium.org Change-Id: Id52deff53913b8d47a4157f42b1fffbd3b103201 Reviewed-on: https://chromium-review.googlesource.com/c/1396094 Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org> Commit-Queue: David Turner <digit@chromium.org> [modify] https://crrev.com/31a486419e279333146ca1e75fcffd510a863d30/presubmit_canned_checks.py [modify] https://crrev.com/31a486419e279333146ca1e75fcffd510a863d30/tests/presubmit_unittest.py
,
Jan 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2e5acea41f0db6f0a98486b7783435e73a8b98d3 commit 2e5acea41f0db6f0a98486b7783435e73a8b98d3 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Fri Jan 11 17:29:56 2019 Roll src/third_party/depot_tools 36248fcd635b..31a486419e27 (1 commits) https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/36248fcd635b..31a486419e27 git log 36248fcd635b..31a486419e27 --date=short --no-merges --format='%ad %ae %s' 2019-01-11 digit@google.com Improve no-long-lines check for Python files. Created with: gclient setdep -r src/third_party/depot_tools@31a486419e27 The AutoRoll server is located here: https://autoroll.skia.org/r/depot-tools-chromium-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. BUG=chromium:890734 TBR=agable@chromium.org Change-Id: I9ab884eab3eec557371b5b10321f893ea09161fd Reviewed-on: https://chromium-review.googlesource.com/c/1406956 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#622046} [modify] https://crrev.com/2e5acea41f0db6f0a98486b7783435e73a8b98d3/DEPS |
||
►
Sign in to add a comment |
||
Comment 1 by ehmaldonado@google.com
, Nov 30