Factor out an input-handling class from Phase in barback.

Factor out an input-handling class from Phase in barback.

In addition to cleaning up Phase, this will make it easier to actually remove
phases when transformers are updated.

R=rnystrom@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=26469

[Bob Nystrom, 2013-08-21 19:47:38]

Couple of suggestions then LGTM.

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/asset_for...
File pkg/barback/lib/src/asset_forwarder.dart (right):

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/asset_for...
pkg/barback/lib/src/asset_forwarder.dart:14: /// having been removed.
Without knowing any larger context, this doesn't explain much to me. Can you
expand on it?

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase.dart
File pkg/barback/lib/src/phase.dart (right):

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase.dar...
pkg/barback/lib/src/phase.dart:104: _onDirtyPool.add(_inputs[node.id].onDirty);
Nit, but how about putting the new PhaseInput in a local variable so you don't
have to keeping looking it up in the map?

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase_inp...
File pkg/barback/lib/src/phase_input.dart (right):

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase_inp...
pkg/barback/lib/src/phase_input.dart:46: /// Theis needs an intervening
controller to ensure that the output can be
"Theis" -> "This"

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase_inp...
pkg/barback/lib/src/phase_input.dart:111: var newTransformers =
transformers.toSet();
How about just making updateTransformers() pass in a Set<T> directly instead of
making each input do the conversion redundantly?

https://codereview.chromium.org/23363002/diff/3001/pkg/barback/lib/src/phase_...
File pkg/barback/lib/src/phase_input.dart (right):

https://codereview.chromium.org/23363002/diff/3001/pkg/barback/lib/src/phase_...
pkg/barback/lib/src/phase_input.dart:20: /// transforms on that node.
Can you clarify here whether it runs transforms where the asset is a primary
input, or *any* input?

[nweiz, 2013-08-21 20:32:51]

Committed patchset #3 manually as r26469 (presubmit successful).

[nweiz, 2013-08-21 20:33:50]

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/asset_for...
File pkg/barback/lib/src/asset_forwarder.dart (right):

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/asset_for...
pkg/barback/lib/src/asset_forwarder.dart:14: /// having been removed.
On 2013/08/21 19:47:38, Bob Nystrom wrote:
> Without knowing any larger context, this doesn't explain much to me. Can you
> expand on it?

Done.

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase.dart
File pkg/barback/lib/src/phase.dart (right):

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase.dar...
pkg/barback/lib/src/phase.dart:104: _onDirtyPool.add(_inputs[node.id].onDirty);
On 2013/08/21 19:47:38, Bob Nystrom wrote:
> Nit, but how about putting the new PhaseInput in a local variable so you don't
> have to keeping looking it up in the map?

Done.

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase_inp...
File pkg/barback/lib/src/phase_input.dart (right):

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase_inp...
pkg/barback/lib/src/phase_input.dart:46: /// Theis needs an intervening
controller to ensure that the output can be
On 2013/08/21 19:47:38, Bob Nystrom wrote:
> "Theis" -> "This"

Done.

https://codereview.chromium.org/23363002/diff/1/pkg/barback/lib/src/phase_inp...
pkg/barback/lib/src/phase_input.dart:111: var newTransformers =
transformers.toSet();
On 2013/08/21 19:47:38, Bob Nystrom wrote:
> How about just making updateTransformers() pass in a Set<T> directly instead
of
> making each input do the conversion redundantly?

Done.

https://codereview.chromium.org/23363002/diff/3001/pkg/barback/lib/src/phase_...
File pkg/barback/lib/src/phase_input.dart (right):

https://codereview.chromium.org/23363002/diff/3001/pkg/barback/lib/src/phase_...
pkg/barback/lib/src/phase_input.dart:20: /// transforms on that node.
On 2013/08/21 19:47:38, Bob Nystrom wrote:
> Can you clarify here whether it runs transforms where the asset is a primary
> input, or *any* input?

Done.