Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2385053003:
Add a basic exact linear macro. Ends up being the same as enum, but making it explicit will make ou… (Closed)
DescriptionAdd a basic exact linear macro. Ends up being the same as enum, but making it explicit will make our API more understandable, as a user shouldn't need to know the detail that enum is actually just 0-1, 1-2, etc.
Also adding mpearson@ and I as owners
BUG=649410
Committed: https://crrev.com/7c83caef73ee8be2d2068c3aff34a11e6d515c44
Cr-Commit-Position: refs/heads/master@{#422971}
Patch Set 1 #Patch Set 2 : whitespace #
Total comments: 3
Patch Set 3 : mpearson comment #Patch Set 4 : add mark and I as owners #Messages
Total messages: 26 (12 generated)
Description was changed from ========== Add a basic exact linear macro. Ends up being the same as enum, but making it explicit will make our API more understandable, as a user shouldn't need to know the detail that enum is actually just 0-1, 1-2, etc. BUG=649410 ========== to ========== Add a basic exact linear macro. Ends up being the same as enum, but making it explicit will make our API more understandable, as a user shouldn't need to know the detail that enum is actually just 0-1, 1-2, etc. BUG=649410 ==========
rkaplow@chromium.org changed reviewers: + mpearson@chromium.org
lgtm with comments --mark https://codereview.chromium.org/2385053003/diff/20001/base/metrics/histogram_... File base/metrics/histogram_macros.h (right): https://codereview.chromium.org/2385053003/diff/20001/base/metrics/histogram_... base/metrics/histogram_macros.h:68: UMA_HISTOGRAM_ENUMERATION(name, sample, value_max) Should we make this value_max+1 to better detect corruption (corrupted data will fall into the overflow bucket, which will not overlap a real bucket)? https://codereview.chromium.org/2385053003/diff/20001/base/metrics/histogram_... base/metrics/histogram_macros.h:70: optional nit: remove one black line
https://codereview.chromium.org/2385053003/diff/20001/base/metrics/histogram_... File base/metrics/histogram_macros.h (right): https://codereview.chromium.org/2385053003/diff/20001/base/metrics/histogram_... base/metrics/histogram_macros.h:68: UMA_HISTOGRAM_ENUMERATION(name, sample, value_max) On 2016/10/03 17:42:29, Mark P wrote: > Should we make this value_max+1 to better detect corruption (corrupted data will > fall into the overflow bucket, which will not overlap a real bucket)? sure, that sounds good, although we do want to do a better job of this more systematically (as per the bug I filed). Made the adjustment here though
still lgtm
The CQ bit was checked by rkaplow@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
The CQ bit was checked by rkaplow@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from ========== Add a basic exact linear macro. Ends up being the same as enum, but making it explicit will make our API more understandable, as a user shouldn't need to know the detail that enum is actually just 0-1, 1-2, etc. BUG=649410 ========== to ========== Add a basic exact linear macro. Ends up being the same as enum, but making it explicit will make our API more understandable, as a user shouldn't need to know the detail that enum is actually just 0-1, 1-2, etc. Also adding mpearson@ and I as owners BUG=649410 ==========
rkaplow@chromium.org changed reviewers: + asvitkine@chromium.org
was meant to be +asvitkine
lgtm
The CQ bit was checked by rkaplow@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mpearson@chromium.org Link to the patchset: https://codereview.chromium.org/2385053003/#ps60001 (title: "add mark and I as owners")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add a basic exact linear macro. Ends up being the same as enum, but making it explicit will make our API more understandable, as a user shouldn't need to know the detail that enum is actually just 0-1, 1-2, etc. Also adding mpearson@ and I as owners BUG=649410 ========== to ========== Add a basic exact linear macro. Ends up being the same as enum, but making it explicit will make our API more understandable, as a user shouldn't need to know the detail that enum is actually just 0-1, 1-2, etc. Also adding mpearson@ and I as owners BUG=649410 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Add a basic exact linear macro. Ends up being the same as enum, but making it explicit will make our API more understandable, as a user shouldn't need to know the detail that enum is actually just 0-1, 1-2, etc. Also adding mpearson@ and I as owners BUG=649410 ========== to ========== Add a basic exact linear macro. Ends up being the same as enum, but making it explicit will make our API more understandable, as a user shouldn't need to know the detail that enum is actually just 0-1, 1-2, etc. Also adding mpearson@ and I as owners BUG=649410 Committed: https://crrev.com/7c83caef73ee8be2d2068c3aff34a11e6d515c44 Cr-Commit-Position: refs/heads/master@{#422971} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/7c83caef73ee8be2d2068c3aff34a11e6d515c44 Cr-Commit-Position: refs/heads/master@{#422971} |
