Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2829823003:
Update and fill up wtf/README.md with a proper introduction to WTF. (Closed)
|
Created:
3 years, 8 months ago by Yuta Kitamura Modified:
3 years, 8 months ago CC:
chromium-reviews, blink-reviews, kinuko+watch Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionUpdate and fill up wtf/README.md with a proper introduction to WTF.
BUG=690809
Review-Url: https://codereview.chromium.org/2829823003
Cr-Commit-Position: refs/heads/master@{#466297}
Committed: https://chromium.googlesource.com/chromium/src/+/f9e23331f466ff10743071637568b517a7214189
Patch Set 1 #
Total comments: 6
Patch Set 2 : Revise the document as per haraken's comments. #Messages
Total messages: 17 (9 generated)
The CQ bit was checked by yutak@chromium.org to run a CQ dry run
yutak@chromium.org changed reviewers: + haraken@chromium.org, tzik@chromium.org
haraken, tzik: PTAL?
Dry run: 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
Dry run: This issue passed the CQ dry run.
LGTM https://codereview.chromium.org/2829823003/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/wtf/README.md (right): https://codereview.chromium.org/2829823003/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/wtf/README.md:10: Dependency-wise, WTF cannot depend on any other Blink modules, including "modules" is confusing with modules/. https://codereview.chromium.org/2829823003/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/wtf/README.md:14: Also let's explain the relationship between base/ and wtf/ more. - We should share as much code between base/ and wtf/ as possible. However, we are not planning to move everything to base/ because Blink needs primitives highly optimized for its workload. Chromium and Blink sometimes have a different assumption about performance, memory and code health. - Since core/ and modules/ cannot use base/, we need to define a thin proxy wrapper in wtf/. https://codereview.chromium.org/2829823003/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/wtf/README.md:80: [Vector]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/wtf/V... I'd prefer dropping these links since it wouldn't be that useful and would be easily out-dated.
lgtm
The CQ bit was checked by yutak@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org, tzik@chromium.org Link to the patchset: https://codereview.chromium.org/2829823003/#ps20001 (title: "Revise the document as per haraken's comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2829823003/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/wtf/README.md (right): https://codereview.chromium.org/2829823003/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/wtf/README.md:10: Dependency-wise, WTF cannot depend on any other Blink modules, including On 2017/04/20 13:45:13, haraken wrote: > > "modules" is confusing with modules/. Done. https://codereview.chromium.org/2829823003/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/wtf/README.md:14: On 2017/04/20 13:45:13, haraken wrote: > > Also let's explain the relationship between base/ and wtf/ more. > > - We should share as much code between base/ and wtf/ as possible. However, we > are not planning to move everything to base/ because Blink needs primitives > highly optimized for its workload. Chromium and Blink sometimes have a different > assumption about performance, memory and code health. > > - Since core/ and modules/ cannot use base/, we need to define a thin proxy > wrapper in wtf/. Done. https://codereview.chromium.org/2829823003/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/wtf/README.md:80: [Vector]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/wtf/V... On 2017/04/20 13:45:13, haraken wrote: > > I'd prefer dropping these links since it wouldn't be that useful and would be > easily out-dated. I would argue that those links are worth having because: (1) they are very useful for those finding a functionality in WTF, and (2) outdated links are only as bad as no links (and fixing is easy).
LGTM
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492763717959200,
"parent_rev": "84111a57f2cbb27474dc475efbcd1a18fae38d4f", "commit_rev":
"f9e23331f466ff10743071637568b517a7214189"}
Message was sent while issue was closed.
Description was changed from ========== Update and fill up wtf/README.md with a proper introduction to WTF. BUG=690809 ========== to ========== Update and fill up wtf/README.md with a proper introduction to WTF. BUG=690809 Review-Url: https://codereview.chromium.org/2829823003 Cr-Commit-Position: refs/heads/master@{#466297} Committed: https://chromium.googlesource.com/chromium/src/+/f9e23331f466ff10743071637568... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/f9e23331f466ff10743071637568... |
