Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(259)

Issue 2581733002: Fix tautological comparison and add clang flag (Closed)

Created:
1 year, 1 month ago by altimin
Modified:
1 year, 1 month ago
Reviewers:
michaeln, brettw, jsbell, tzik
CC:
chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix tautological comparison and add clang flag Committed: https://crrev.com/1b142eb67aedcd40f07234e38174b5b0f9995dc5 Cr-Commit-Position: refs/heads/master@{#439346}

Patch Set 1 #

Total comments: 3

Patch Set 2 : Remove warnings from openh264 #

Patch Set 3 : Remove clang warnings only when using clang #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -3 lines) Patch
M build/config/clang/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M storage/browser/quota/quota_manager.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M third_party/openh264/BUILD.gn View 1 2 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 39 (24 generated)
altimin
PTAL
1 year, 1 month ago (2016-12-15 17:55:04 UTC) #2
brettw
build owners lgtm, but please check with Michael before checking in. https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc File storage/browser/quota/quota_manager.cc (right): ...
1 year, 1 month ago (2016-12-15 18:49:31 UTC) #3
altimin
michaeln@, could you PTAL? https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc File storage/browser/quota/quota_manager.cc (right): https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc#newcode89 storage/browser/quota/quota_manager.cc:89: return type == kStorageTypeTemporary || ...
1 year, 1 month ago (2016-12-15 18:57:08 UTC) #5
michaeln
https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc File storage/browser/quota/quota_manager.cc (right): https://codereview.chromium.org/2581733002/diff/1/storage/browser/quota/quota_manager.cc#newcode89 storage/browser/quota/quota_manager.cc:89: return type == kStorageTypeTemporary || type == kStorageTypePersistent; On ...
1 year, 1 month ago (2016-12-15 19:26:55 UTC) #10
michaeln
> I wouldn't have expected persistent storage > to be allowed in incognito. is is ...
1 year, 1 month ago (2016-12-15 19:29:40 UTC) #11
altimin
brettw@, there were failures in openh264, I disabled warnings for that third-party library. Could you ...
1 year, 1 month ago (2016-12-15 20:51:19 UTC) #12
brettw
lgtm
1 year, 1 month ago (2016-12-15 22:10:45 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581733002/30001
1 year, 1 month ago (2016-12-16 19:00:42 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel_ng/builds/281306) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
1 year, 1 month ago (2016-12-16 22:21:33 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581733002/30001
1 year, 1 month ago (2016-12-16 23:43:07 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/358626)
1 year, 1 month ago (2016-12-17 03:46:26 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2581733002/30001
1 year, 1 month ago (2016-12-17 14:23:12 UTC) #32
commit-bot: I haz the power
Committed patchset #3 (id:30001)
1 year, 1 month ago (2016-12-17 18:51:37 UTC) #35
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/1b142eb67aedcd40f07234e38174b5b0f9995dc5 Cr-Commit-Position: refs/heads/master@{#439346}
1 year, 1 month ago (2016-12-17 18:54:21 UTC) #37
jsbell
1 year, 1 month ago (2016-12-20 19:46:35 UTC) #39
Message was sent while issue was closed.
FYI: pending reversion for the base CL at
https://codereview.chromium.org/2592793002

That'll remove the tautological statement; we'll need to make sure this fix gets
reapplied when it relands although the clang flag should catch it.

Powered by Google App Engine
This is Rietveld 408576698