Fix clang/win build after https://codereview.chromium.org/1543173002/

Fix clang/win build after https://codereview.chromium.org/1543173002/

That change made metrics_aura.cc include build_config.h.  Before,
nothing did, so `defined(OS_WIN)` was always false (even on Windows!).
Now that it isn't, clang-cl complains that a constant is unused
on Windows, so move it to a more local scope.

BUG=82385
R=tfarina@chromium.org
TBR=avi

Committed: https://crrev.com/3f0a8e7542c9d6824eb34bcdcc8f4b1738df2f08
Cr-Commit-Position: refs/heads/master@{#366771}

[Nico, 2015-12-23 21:19:31]

Description was changed from

==========
Fix clang/win build after https://codereview.chromium.org/1543173002/

That change made metrics_aura.cc include build_config.h.  Before,
nothing did, so `defined(OS_WIN)` was always false.  Now that it isn't,
clang-cl complains that a constant is unused on Windows, so move it
to a more local scope.

BUG=82385
==========

to

==========
Fix clang/win build after https://codereview.chromium.org/1543173002/

That change made metrics_aura.cc include build_config.h.  Before,
nothing did, so `defined(OS_WIN)` was always false.  Now that it isn't,
clang-cl complains that a constant is unused on Windows, so move it
to a more local scope.

BUG=82385
TBR=avi
==========

[Nico, 2015-12-23 21:19:40]

thakis@chromium.org changed reviewers:
+ avi@chromium.org

[Nico, 2015-12-23 21:19:44]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2015-12-23 21:19:52]

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

[tfarina, 2015-12-23 21:23:24]

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

[tfarina, 2015-12-23 21:23:25]

https://codereview.chromium.org/1544083002/diff/1/ui/views/metrics_aura.cc
File ui/views/metrics_aura.cc (right):

https://codereview.chromium.org/1544083002/diff/1/ui/views/metrics_aura.cc#ne...
ui/views/metrics_aura.cc:19: const int kDefaultDoubleClickIntervalMs = 500;
right, on Windows, this constant was unused. Good catch of clang-cl!

lgtm

[Nico, 2015-12-23 21:25:30]

Description was changed from

==========
Fix clang/win build after https://codereview.chromium.org/1543173002/

That change made metrics_aura.cc include build_config.h.  Before,
nothing did, so `defined(OS_WIN)` was always false.  Now that it isn't,
clang-cl complains that a constant is unused on Windows, so move it
to a more local scope.

BUG=82385
TBR=avi
==========

to

==========
Fix clang/win build after https://codereview.chromium.org/1543173002/

That change made metrics_aura.cc include build_config.h.  Before,
nothing did, so `defined(OS_WIN)` was always false (even on Windows!).
Now that it isn't, clang-cl complains that a constant is unused
on Windows, so move it to a more local scope.

BUG=82385
TBR=avi
==========

[commit-bot: I haz the power, 2015-12-23 21:32:55]

Description was changed from

==========
Fix clang/win build after https://codereview.chromium.org/1543173002/

That change made metrics_aura.cc include build_config.h.  Before,
nothing did, so `defined(OS_WIN)` was always false (even on Windows!).
Now that it isn't, clang-cl complains that a constant is unused
on Windows, so move it to a more local scope.

BUG=82385
TBR=avi
==========

to

==========
Fix clang/win build after https://codereview.chromium.org/1543173002/

That change made metrics_aura.cc include build_config.h.  Before,
nothing did, so `defined(OS_WIN)` was always false (even on Windows!).
Now that it isn't, clang-cl complains that a constant is unused
on Windows, so move it to a more local scope.

BUG=82385
R=tfarina@chromium.org
TBR=avi

Committed: https://crrev.com/3f0a8e7542c9d6824eb34bcdcc8f4b1738df2f08
Cr-Commit-Position: refs/heads/master@{#366771}
==========

[commit-bot: I haz the power, 2015-12-23 21:32:56]

Patchset 1 (id:??) landed as
https://crrev.com/3f0a8e7542c9d6824eb34bcdcc8f4b1738df2f08
Cr-Commit-Position: refs/heads/master@{#366771}

[Avi (use Gerrit), 2015-12-24 00:35:39]

Nice!

And this is what upsets me about all of this. Everyone was using OS_whatever and
never including the header files. Ugh.

[Avi (use Gerrit), 2015-12-24 00:35:57]

LGTM

[Avi (use Gerrit), 2015-12-24 01:48:13]

On 2015/12/24 00:35:57, Avi wrote:
> LGTM

BTW, should we presubmit this? If you use an OS_ macro, you have to include
build_config.h above it?

[Nico, 2015-12-24 02:04:41]

I'm generally not a huge fan of presubmits that try to parse code. This
could be implemented by using cleverer preprocessor macros (see e.g.
build/write_buildflag_header.py and build/buildflag.h, or the corresponding
OS() and USE() macros in webkit) -- if this is a common problem, we should
do something like that instead.

On Wed, Dec 23, 2015 at 8:48 PM, <avi@chromium.org> wrote:

> On 2015/12/24 00:35:57, Avi wrote:
>
>> LGTM
>>
>
> BTW, should we presubmit this? If you use an OS_ macro, you have to include
> build_config.h above it?
>
> https://codereview.chromium.org/1544083002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Avi (use Gerrit), 2015-12-24 02:40:20]

I think everyone assumes that OS_* is done with a -D flag on the compiler. I
don't think anyone realizes you need a header file...