Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(25)

Issue 21275003: Move barback to a more event-based model. (Closed)

Created:
7 years, 7 months ago by nweiz
Modified:
7 years, 7 months ago
Reviewers:
Bob Nystrom
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Move barback to a more event-based model. The goal of this change is to make it easy to synchronously propagate the dirty state of an asset node to all of the nodes and transforms that depend on it, while still asynchronously processing those transforms. In addition, this refactor ends up addressing two TODOs and one bug: * If a node has been compiled previously and an unrelated part of its cascade is dirtied, it will now be returned immediately rather than waiting for the entire cascade to finish processing. * If a node is consumed as the input of a transform, it will no longer be returned by [getAssetById]. * If the contents of a transform's primary input changes such that it's no longer a valid primary input for that transform, the transform will be removed. R=rnystrom@google.com Committed: https://code.google.com/p/dart/source/detail?r=25693

Patch Set 1 #

Total comments: 67

Patch Set 2 : Code review changes. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1033 lines, -250 lines) Patch
M pkg/barback/lib/src/asset_cascade.dart View 1 7 chunks +87 lines, -67 lines 0 comments Download
M pkg/barback/lib/src/asset_node.dart View 1 1 chunk +174 lines, -15 lines 0 comments Download
M pkg/barback/lib/src/package_provider.dart View 1 1 chunk +4 lines, -0 lines 0 comments Download
M pkg/barback/lib/src/phase.dart View 1 3 chunks +202 lines, -104 lines 0 comments Download
M pkg/barback/lib/src/transform.dart View 1 chunk +11 lines, -3 lines 0 comments Download
M pkg/barback/lib/src/transform_node.dart View 1 1 chunk +114 lines, -57 lines 0 comments Download
M pkg/barback/test/package_graph/errors_test.dart View 4 chunks +27 lines, -4 lines 0 comments Download
M pkg/barback/test/package_graph/source_test.dart View 2 chunks +19 lines, -0 lines 0 comments Download
M pkg/barback/test/package_graph/transform_test.dart View 1 5 chunks +395 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
nweiz
7 years, 7 months ago (2013-07-31 00:12:13 UTC) #1
Bob Nystrom
Both barback before your patch and with it still intuitively feels more complex than it ...
7 years, 7 months ago (2013-07-31 20:05:41 UTC) #2
nweiz
https://codereview.chromium.org/21275003/diff/1/pkg/barback/lib/src/asset_cascade.dart File pkg/barback/lib/src/asset_cascade.dart (right): https://codereview.chromium.org/21275003/diff/1/pkg/barback/lib/src/asset_cascade.dart#newcode43 pkg/barback/lib/src/asset_cascade.dart:43: /// cascade's package's source assets, indexed by id. On ...
7 years, 7 months ago (2013-07-31 22:47:53 UTC) #3
Bob Nystrom
LGTM! https://codereview.chromium.org/21275003/diff/1/pkg/barback/lib/src/transform_node.dart File pkg/barback/lib/src/transform_node.dart (right): https://codereview.chromium.org/21275003/diff/1/pkg/barback/lib/src/transform_node.dart#newcode163 pkg/barback/lib/src/transform_node.dart:163: var controller = _outputControllers[asset.id]; On 2013/07/31 22:47:53, nweiz ...
7 years, 7 months ago (2013-07-31 23:20:15 UTC) #4
nweiz
7 years, 7 months ago (2013-07-31 23:51:46 UTC) #5
Message was sent while issue was closed.
Committed patchset #2 manually as r25693 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698