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

Issue 967243002: Polish Maybe API a bit, removing useless creativity and fixing some signatures. (Closed)

Created:
5 years, 9 months ago by Sven Panne
Modified:
4 years, 10 months ago
Reviewers:
dcarney, Nico
CC:
v8-dev, Paweł Hajdan Jr.
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Polish Maybe API a bit, removing useless creativity and fixing some signatures. BUG=v8:3929 LOG=y R=dcarney@chromium.org Committed: https://crrev.com/30637108dd3977884cb81741bec248c9eb22f4e3 Cr-Commit-Position: refs/heads/master@{#26936}

Patch Set 1 #

Patch Set 2 : Simplified friendship. Added check in FromJust. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+147 lines, -142 lines) Patch
M include/v8.h View 1 4 chunks +56 lines, -41 lines 1 comment Download
M src/api.cc View 1 10 chunks +31 lines, -34 lines 0 comments Download
M src/ast.h View 1 chunk +1 line, -2 lines 0 comments Download
M src/contexts.cc View 3 chunks +4 lines, -5 lines 0 comments Download
M src/ic/ic-state.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/ic/ic-state.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/objects.cc View 22 chunks +46 lines, -48 lines 0 comments Download
M src/objects-inl.h View 4 chunks +4 lines, -8 lines 0 comments Download
M src/runtime/runtime-object.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/type-info.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/typing.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 10 (4 generated)
Sven Panne
Strike while the iron is hot... ;-)
5 years, 9 months ago (2015-03-02 09:10:38 UTC) #2
dcarney
lgtm
5 years, 9 months ago (2015-03-02 10:09:13 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/967243002/20001
5 years, 9 months ago (2015-03-02 10:20:53 UTC) #5
Sven Panne
Committed patchset #2 (id:20001) manually as 30637108dd3977884cb81741bec248c9eb22f4e3 (presubmit successful).
5 years, 9 months ago (2015-03-02 11:27:12 UTC) #7
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/30637108dd3977884cb81741bec248c9eb22f4e3 Cr-Commit-Position: refs/heads/master@{#26936}
5 years, 9 months ago (2015-03-02 11:27:15 UTC) #8
Nico
4 years, 10 months ago (2016-02-26 19:18:26 UTC) #10
Message was sent while issue was closed.
https://codereview.chromium.org/967243002/diff/20001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/967243002/diff/20001/include/v8.h#newcode5737
include/v8.h:5737: V8_INLINE bool IsJust() const { return has_value; }
I saw this since someone made a similar name change in Blink. I think this is a
way worse name than the "uselessly creative" name we had before. People who know
Haskell will get "isJust" but nobody else will without looking it up. "HasValue"
is immediately understandable. Most people don't know Haskell.

Powered by Google App Engine
This is Rietveld 408576698