Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1039)

Issue 29390650: Issue 5020 - Part 1: Make compile script PEP8-compliant (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 8 months ago by Wladimir Palant
Modified:
2 years, 7 months ago
CC:
hub
Base URL:
https://hg.adblockplus.org/adblockpluscore
Visibility:
Public.

Description

Issue 5020 - Part 1: Make compile script PEP8-compliant

Patch Set 1 #

Total comments: 6

Patch Set 2 : Fixed string formatting #

Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -53 lines) Patch
M compile View 1 1 chunk +58 lines, -53 lines 0 comments Download

Messages

Total messages: 8
Wladimir Palant
2 years, 8 months ago (2017-03-21 11:22:02 UTC) #1
Vasily Kuznetsov
Hi Wladimir, The changes look good but I also wanted to run flake8 over them ...
2 years, 8 months ago (2017-03-21 12:33:57 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29390650/diff/29390651/compile File compile (right): https://codereview.adblockplus.org/29390650/diff/29390651/compile#newcode42 compile:42: '/bin/bash', '-c', os.path.join(EMSCRIPTEN_PATH, 'emsdk_env.sh') Unrelated of coding style changes ...
2 years, 8 months ago (2017-03-21 12:41:01 UTC) #3
Wladimir Palant
On 2017/03/21 12:33:57, Vasily Kuznetsov wrote: > The changes look good but I also wanted ...
2 years, 8 months ago (2017-03-21 13:21:55 UTC) #4
Wladimir Palant
https://codereview.adblockplus.org/29390650/diff/29390651/compile File compile (right): https://codereview.adblockplus.org/29390650/diff/29390651/compile#newcode42 compile:42: '/bin/bash', '-c', os.path.join(EMSCRIPTEN_PATH, 'emsdk_env.sh') On 2017/03/21 12:41:01, Sebastian Noack ...
2 years, 8 months ago (2017-03-21 13:33:11 UTC) #5
Vasily Kuznetsov
https://codereview.adblockplus.org/29390650/diff/29390651/compile File compile (right): https://codereview.adblockplus.org/29390650/diff/29390651/compile#newcode20 compile:20: 'SHELL_FILE': "'%s'" % os.path.abspath(os.path.join(SOURCE_DIR, Funnily enough here flake8-abp doesn't ...
2 years, 8 months ago (2017-03-21 13:49:35 UTC) #6
Vasily Kuznetsov
LGTM for patch set 2
2 years, 8 months ago (2017-03-21 13:50:36 UTC) #7
Sebastian Noack
2 years, 8 months ago (2017-03-21 14:12:55 UTC) #8
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5