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

Issue 386933002: [BuildBreak] Linux: Fix g++ 4.8 compilation for error: multi-line comment [-Werror=comment] (Closed)

Created:
6 years, 5 months ago by vivekg_samsung
Modified:
6 years, 5 months ago
CC:
chromium-reviews, vivekg
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

[BuildBreak] Linux: Fix g++ 4.8 compilation for error: multi-line comment [-Werror=comment] While compling on linux with g++ 4.8, the file chrome/test/ppapi/ppapi_browsertest.cc is throwing ../../chrome/test/ppapi/ppapi_browsertest.cc:452:1: error: multi-line comment [-Werror=comment] cc1plus: all warnings being treated as errors NOTRY=true Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282597

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M chrome/test/ppapi/ppapi_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 10 (0 generated)
vivekg
PTAL, thank you! @thakis, is this something that should also be enabled while using clang ...
6 years, 5 months ago (2014-07-11 08:47:59 UTC) #1
yhirano1
lgtm, thanks
6 years, 5 months ago (2014-07-11 09:13:47 UTC) #2
vivekg
The CQ bit was checked by vivekg@chromium.org
6 years, 5 months ago (2014-07-11 09:14:13 UTC) #3
yhirano
lgtm
6 years, 5 months ago (2014-07-11 09:14:14 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/386933002/1
6 years, 5 months ago (2014-07-11 09:14:29 UTC) #5
vivekg
The CQ bit was unchecked by vivekg@chromium.org
6 years, 5 months ago (2014-07-11 09:15:28 UTC) #6
vivekg
The CQ bit was checked by vivekg@chromium.org
6 years, 5 months ago (2014-07-11 09:15:42 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vivek.vg@samsung.com/386933002/1
6 years, 5 months ago (2014-07-11 09:15:47 UTC) #8
commit-bot: I haz the power
Change committed as 282597
6 years, 5 months ago (2014-07-11 09:36:11 UTC) #9
Nico
6 years, 5 months ago (2014-07-12 19:02:59 UTC) #10
On Fri, Jul 11, 2014 at 1:47 AM, <vivekg@chromium.org> wrote:

> PTAL, thank you!
>

Thank you for the fix!


> @thakis, is this something that should also be enabled while using clang as
> default toolchain on linux?
>

My understand was that it should already be enabled. Clang has a -Wcomment,
and it seems to work:

$ cat test.cc
//      LIST_TEST(URLLoader_UntrustedHttpRequests) \

int a;
$ clang -Wall test.cc -c
test.cc:1:52: warning: multi-line // comment [-Wcomment]
//      LIST_TEST(URLLoader_UntrustedHttpRequests) \
                                                   ^
1 warning generated.

I checked that we do build browser_tests with -Wall. So I'm not sure why
our bots didn't catch this. Maybe you can try reverting this locally and
check if clang catches it, and if not, make a reduced test case and send
that to me?

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698