Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(902)

Issue 12255083: Fix PyLint presbumit check on win. (Closed)

Created:
7 years, 10 months ago by robertshield
Modified:
7 years, 10 months ago
CC:
chromium-reviews, Dirk Pranke, cmp+cc_chromium.org, M-A Ruel, iannucci+depot_tools_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/tools/depot_tools.git@master
Visibility:
Public.

Description

Fix PyLint presbumit check on win. We were passing a unicode string in the env block to subprocess.Popen which makes it unhappy. This forces the string to ascii first. BUG=NONE TEST=NONE Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=183569

Patch Set 1 : #

Total comments: 2

Patch Set 2 : Dear Marc-Antoine, #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M presubmit_canned_checks.py View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
robertshield
In case you saw a couple of git cl crash dumps in PyLint come by ...
7 years, 10 months ago (2013-02-19 20:57:02 UTC) #1
M-A Ruel
https://codereview.chromium.org/12255083/diff/2001/presubmit_canned_checks.py File presubmit_canned_checks.py (right): https://codereview.chromium.org/12255083/diff/2001/presubmit_canned_checks.py#newcode680 presubmit_canned_checks.py:680: extra_paths_list + sys.path).encode('ascii', 'ignore') utf8 technically we'd want the ...
7 years, 10 months ago (2013-02-19 21:07:40 UTC) #2
robertshield
ptal https://codereview.chromium.org/12255083/diff/2001/presubmit_canned_checks.py File presubmit_canned_checks.py (right): https://codereview.chromium.org/12255083/diff/2001/presubmit_canned_checks.py#newcode680 presubmit_canned_checks.py:680: extra_paths_list + sys.path).encode('ascii', 'ignore') On 2013/02/19 21:07:40, Marc-Antoine ...
7 years, 10 months ago (2013-02-20 15:23:37 UTC) #3
Marc-Antoine Ruel (Google)
lgtm
7 years, 10 months ago (2013-02-20 18:05:08 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/robertshield@chromium.org/12255083/5001
7 years, 10 months ago (2013-02-20 18:06:07 UTC) #5
commit-bot: I haz the power
7 years, 10 months ago (2013-02-20 18:08:48 UTC) #6
Message was sent while issue was closed.
Change committed as 183569

Powered by Google App Engine
This is Rietveld 408576698