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

Issue 2567943004: Rename blink::Document::ReadyState enum to DocumentReadyState. (Closed)

Created:
4 years ago by Łukasz Anforowicz
Modified:
4 years ago
Reviewers:
esprehn
CC:
blink-reviews, blink-reviews-dom_chromium.org, chromium-reviews, dcheng, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Rename blink::Document::ReadyState enum to DocumentReadyState. The rename is needed to avoid a naming collision after changing from Blink to Chromium naming style. Right now we have a |ReadyState| type and a |readyState| accessor method (differing by case of the first character); after a naive rename by the rewrite_to_chrome_style tool we would end up with |ReadyState| being the name of both the type and the accessor method (with both living in the same namespace). Prepending a "get" prefix to the name of the accessor method is the workaround that fits into the guidance on the recommended post-Blink-to-Chromium-rename style suggested by esprehn@ in https://crbug.com/582312#c17: - Getters favor not using "Get", ex. FirstChild() - Unless the type name conflicts, in which case you can either rename the type if it's easy and makes sense, or add "Get", ex. GetContext(). FWIW, this rename brings the name of the enum closer to the name of the enum in the WHATWG spec. BUG=582312 Committed: https://crrev.com/6b090f5776dec4e955552c8e46f2bed21e3a9cff Cr-Commit-Position: refs/heads/master@{#438018}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -4 lines) Patch
M third_party/WebKit/Source/core/dom/Document.h View 2 chunks +6 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Document.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 17 (10 generated)
Łukasz Anforowicz
esprehn@ - can you PTAL? (this CL is an offshoot from https://codereview.chromium.org/2572603002)
4 years ago (2016-12-13 00:14:47 UTC) #4
esprehn
lgtm
4 years ago (2016-12-13 00:24:20 UTC) #5
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/2567943004/1
4 years ago (2016-12-13 00:25:37 UTC) #8
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/278138) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
4 years ago (2016-12-13 01:35:43 UTC) #10
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/2567943004/1
4 years ago (2016-12-13 01:41:11 UTC) #12
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years ago (2016-12-13 03:15:30 UTC) #15
commit-bot: I haz the power
4 years ago (2016-12-13 03:18:49 UTC) #17
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/6b090f5776dec4e955552c8e46f2bed21e3a9cff
Cr-Commit-Position: refs/heads/master@{#438018}

Powered by Google App Engine
This is Rietveld 408576698