Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2787843004:
Attempt to simplify source_index calculation (Closed)
DescriptionSimplify source_index and source_offset calculation
The previous calculation is equivalent to looking up Parent(layer) for
each branch and variable.
BUG=707217
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Review-Url: https://codereview.chromium.org/2787843004
Cr-Commit-Position: refs/heads/master@{#461694}
Committed: https://chromium.googlesource.com/chromium/src/+/beeaae78ce679a18605c63591ec6532e304b3d9e
Patch Set 1 #Patch Set 2 : Cleanup #
Total comments: 2
Patch Set 3 : Change source_index initialization #Messages
Total messages: 25 (18 generated)
Description was changed from ========== Attempt to simplify source_index calculation BUG= ========== to ========== Attempt to simplify source_index calculation BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run
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.
+vollick, weiliangc as an FYI; looks like our analysis of source_index was reasonably likely correct. I will double-check that there are appropriate tests already in place, add any if necessary, and send this out for full review soon.
Description was changed from ========== Attempt to simplify source_index calculation BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Simplify source_index and source_offset calculation The previous calculation is equivalent to looking up Parent(layer) for each branch and variable. BUG=707217 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
smcgruer@chromium.org changed reviewers: + ajuma@chromium.org, vollick@chromium.org, weiliangc@chromium.org
The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LGTM https://codereview.chromium.org/2787843004/diff/20001/cc/trees/property_tree_... File cc/trees/property_tree_builder.cc (right): https://codereview.chromium.org/2787843004/diff/20001/cc/trees/property_tree_... cc/trees/property_tree_builder.cc:541: int source_index = parent_index; nit: I think it's cleaner if source_index is initialized to kRootNodeId too.
smcgruer@chromium.org changed required reviewers: + ajuma@chromium.org, weiliangc@chromium.org
https://codereview.chromium.org/2787843004/diff/20001/cc/trees/property_tree_... File cc/trees/property_tree_builder.cc (right): https://codereview.chromium.org/2787843004/diff/20001/cc/trees/property_tree_... cc/trees/property_tree_builder.cc:541: int source_index = parent_index; On 2017/03/31 17:44:48, weiliangc wrote: > nit: I think it's cleaner if source_index is initialized to kRootNodeId too. Done.
The CQ bit was checked by smcgruer@chromium.org to run a CQ dry run
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 too, thanks for simplifying this logic!
The CQ bit was checked by smcgruer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from weiliangc@chromium.org Link to the patchset: https://codereview.chromium.org/2787843004/#ps40001 (title: "Change source_index initialization")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491312141341120,
"parent_rev": "5f7862ecfcdcdf42328842d94b7eaf039bd2e839", "commit_rev":
"beeaae78ce679a18605c63591ec6532e304b3d9e"}
Message was sent while issue was closed.
Description was changed from ========== Simplify source_index and source_offset calculation The previous calculation is equivalent to looking up Parent(layer) for each branch and variable. BUG=707217 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== Simplify source_index and source_offset calculation The previous calculation is equivalent to looking up Parent(layer) for each branch and variable. BUG=707217 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2787843004 Cr-Commit-Position: refs/heads/master@{#461694} Committed: https://chromium.googlesource.com/chromium/src/+/beeaae78ce679a18605c63591ec6... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/beeaae78ce679a18605c63591ec6... |
