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

Issue 1252913006: Fix chromium style checker error on windows sandbox code. (Closed)

Created:
5 years, 4 months ago by Yoshisato Yanagisawa
Modified:
5 years, 4 months ago
Reviewers:
Nico, *Will Harris
CC:
chromium-reviews, wfh+watch_chromium.org, rickyz+watch_chromium.org, rvargas (doing something else)
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix chromium style checker error on windows sandbox code. This change fixes "Complex constructor has an inline body." error generated by Windows clang. If you set clang=1 in GYP_DEFINES, you can easily reproduce this. TBR=wfh

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -3 lines) Patch
M sandbox/win/src/Wow64.h View 1 chunk +1 line, -2 lines 0 comments Download
M sandbox/win/src/Wow64.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M sandbox/win/src/job.h View 1 chunk +1 line, -1 line 0 comments Download
M sandbox/win/src/job.cc View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (6 generated)
Yoshisato Yanagisawa
Hi, I found that these code cannot compiled with Windows clang because of style error: ...
5 years, 4 months ago (2015-07-28 08:36:03 UTC) #3
Nico
Lgtm, thanks. Since this fixes bots, I think it's OK to tbr this.
5 years, 4 months ago (2015-07-28 13:30:46 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1252913006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1252913006/1
5 years, 4 months ago (2015-07-28 13:31:37 UTC) #6
commit-bot: I haz the power
All required reviewers (with asterisk prefixes) have not yet approved this CL. No L-G-T-M from ...
5 years, 4 months ago (2015-07-28 13:31:40 UTC) #8
Will Harris
lgtm
5 years, 4 months ago (2015-07-28 14:54:05 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1252913006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1252913006/1
5 years, 4 months ago (2015-07-28 14:54:24 UTC) #11
Nico
On 2015/07/28 13:31:40, commit-bot: I haz the power wrote: > All required reviewers (with asterisk ...
5 years, 4 months ago (2015-07-28 14:58:27 UTC) #12
Will Harris
On 2015/07/28 14:58:27, Nico (vacation Mon Jul 27) wrote: > On 2015/07/28 13:31:40, commit-bot: I ...
5 years, 4 months ago (2015-07-28 15:02:59 UTC) #13
Nico
On 2015/07/28 15:02:59, Will Harris wrote: > On 2015/07/28 14:58:27, Nico (vacation Mon Jul 27) ...
5 years, 4 months ago (2015-07-28 15:15:20 UTC) #14
Nico
I guess sandbox/win/src/Wow64_64.cc needs the out of line constructor too?
5 years, 4 months ago (2015-07-28 15:17:52 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/85345)
5 years, 4 months ago (2015-07-28 15:23:09 UTC) #17
Nico
Looks like I can't upload to this issue. Cloned this CL to https://codereview.chromium.org/1257963003 and edited ...
5 years, 4 months ago (2015-07-28 15:24:45 UTC) #18
Yoshisato Yanagisawa
I am not a committer. I will not set required reviewers for this kind of ...
5 years, 4 months ago (2015-07-29 00:58:59 UTC) #19
Yoshisato Yanagisawa
5 years, 4 months ago (2015-07-29 00:59:44 UTC) #20
Message was sent while issue was closed.
note: this change has already been taken over by
https://codereview.chromium.org/1257963003.  Let me close this change list.

Powered by Google App Engine
This is Rietveld 408576698