Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1659323003:
Convert base/numerics conversions to constexpr (Closed)
|
Created:
4 years, 10 months ago by jschuh Modified:
4 years, 8 months ago Reviewers:
brucedawson CC:
chromium-reviews, vmpstr+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd constexpr to base/numerics conversion functions
Committed: https://crrev.com/83882396f04fb36ffe27caef98f623e1a3bcac8b
Cr-Commit-Position: refs/heads/master@{#388588}
Patch Set 1 #Patch Set 2 : moar constexpr #Patch Set 3 : assert #
Total comments: 1
Patch Set 4 : nit #
Messages
Total messages: 22 (10 generated)
Patchset #2 (id:20001) has been deleted
jschuh@chromium.org changed reviewers: + brucedawson@chromium.org
I'm queuing these up now that MSVS 2015 is about to land.
Description was changed from ========== Add constexpr to base/numerics conversions ========== to ========== Add constexpr to base/numerics conversion functions ==========
lgtm, but we should probably hold off until a week or so after the transition, to avoid making possible reverts painful. https://codereview.chromium.org/1659323003/diff/60001/base/numerics/safe_conv... File base/numerics/safe_conversions_impl.h (right): https://codereview.chromium.org/1659323003/diff/60001/base/numerics/safe_conv... base/numerics/safe_conversions_impl.h:8: #include <assert.h> Delete - presumably not needed?
The CQ bit was checked by jschuh@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from brucedawson@chromium.org Link to the patchset: https://codereview.chromium.org/1659323003/#ps80001 (title: "nit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1659323003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1659323003/80001
On 2016/03/18 23:13:55, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > https://chromium-cq-status.appspot.com/patch-status/1659323003/80001 > View timeline at > https://chromium-cq-status.appspot.com/patch-timeline/1659323003/80001 Is this going to require VS 2015? We're still working through some DrMemory bugs and one temporary mitigation was to revert those builders back to VS 2013. Tradeoffs...
The CQ bit was unchecked by jschuh@chromium.org
On 2016/03/18 23:17:40, brucedawson wrote: > On 2016/03/18 23:13:55, commit-bot: I haz the power wrote: > > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/patch-status/1659323003/80001 > > View timeline at > > https://chromium-cq-status.appspot.com/patch-timeline/1659323003/80001 > > Is this going to require VS 2015? > > We're still working through some DrMemory bugs and one temporary mitigation was > to revert those builders back to VS 2013. Tradeoffs... Well then, holding off for now.
The CQ bit was checked by jschuh@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1659323003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1659323003/80001
On 2016/04/20 17:54:18, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > https://chromium-cq-status.appspot.com/patch-status/1659323003/80001 > View timeline at > https://chromium-cq-status.appspot.com/patch-timeline/1659323003/80001 Welcome to the future.
On 2016/04/20 18:35:01, brucedawson wrote: > On 2016/04/20 17:54:18, commit-bot: I haz the power wrote: > > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/patch-status/1659323003/80001 > > View timeline at > > https://chromium-cq-status.appspot.com/patch-timeline/1659323003/80001 > > Welcome to the future. If this lands clean -- which it looks like it should -- it will be the oldest unmodified CL I've ever pushed. And it's older than it looks, because it sat in a local branch for a few months before I posted it.
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
The CQ bit was checked by jschuh@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1659323003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1659323003/80001
Message was sent while issue was closed.
Committed patchset #4 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Add constexpr to base/numerics conversion functions ========== to ========== Add constexpr to base/numerics conversion functions Committed: https://crrev.com/83882396f04fb36ffe27caef98f623e1a3bcac8b Cr-Commit-Position: refs/heads/master@{#388588} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/83882396f04fb36ffe27caef98f623e1a3bcac8b Cr-Commit-Position: refs/heads/master@{#388588} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
