Subprocess uses shell=True on Windows when launching GN.

Subprocess uses shell=True on Windows when launching GN.

BUG=600051
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300311

[Bret, 2016-04-28 23:41:05]

Description was changed from

==========
Subprocess uses shell=True on Windows when launching GN.

BUG=600051
==========

to

==========
Subprocess uses shell=True on Windows when launching GN.

BUG=600051
==========

[Bret, 2016-04-28 23:41:05]

bsep@chromium.org changed reviewers:
+ iannucci@chromium.org

[scottmg, 2016-04-28 23:49:43]

scottmg@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2016-04-28 23:49:44]

lgtm

https://codereview.chromium.org/1917863009/diff/1/git_cl.py
File git_cl.py (right):

https://codereview.chromium.org/1917863009/diff/1/git_cl.py#newcode4752
git_cl.py:4752: cwd=top_dir,
Swap the order of cwd= and shell=, since shell= is going into a named argument
and cwd= is going into **kwargs.

https://codereview.chromium.org/1917863009/diff/1/git_cl.py#newcode4753
git_cl.py:4753: shell=(sys.platform == 'win32'))
nit; I would remove the parens around (sys.platform == 'win32').

[iannucci, 2016-04-28 23:50:37]

lgtm

[Bret, 2016-04-28 23:57:16]

https://codereview.chromium.org/1917863009/diff/1/git_cl.py
File git_cl.py (right):

https://codereview.chromium.org/1917863009/diff/1/git_cl.py#newcode4752
git_cl.py:4752: cwd=top_dir,
On 2016/04/28 23:49:43, scottmg wrote:
> Swap the order of cwd= and shell=, since shell= is going into a named argument
> and cwd= is going into **kwargs.

Done.

https://codereview.chromium.org/1917863009/diff/1/git_cl.py#newcode4753
git_cl.py:4753: shell=(sys.platform == 'win32'))
On 2016/04/28 23:49:43, scottmg wrote:
> nit; I would remove the parens around (sys.platform == 'win32').

Done.

[Bret, 2016-04-28 23:57:50]

The CQ bit was checked by bsep@chromium.org

[Bret, 2016-04-28 23:57:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org, scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/1917863009/#ps20001
(title: "fix nits")

[commit-bot: I haz the power, 2016-04-28 23:58:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1917863009/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1917863009/20001

[commit-bot: I haz the power, 2016-04-29 00:00:53]

Description was changed from

==========
Subprocess uses shell=True on Windows when launching GN.

BUG=600051
==========

to

==========
Subprocess uses shell=True on Windows when launching GN.

BUG=600051

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=300311
==========

[commit-bot: I haz the power, 2016-04-29 00:00:54]

Committed patchset #2 (id:20001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=300311

[tandrii(chromium), 2016-04-29 12:46:18]

tandrii@chromium.org changed reviewers:
+ tandrii@chromium.org

[tandrii(chromium), 2016-04-29 12:46:18]

LGTM