Split AssetGraph.results into two streams.

pkg/barback/lib/src/asset_graph.dart

Index: pkg/barback/lib/src/asset_graph.dart
diff --git a/pkg/barback/lib/src/asset_graph.dart b/pkg/barback/lib/src/asset_graph.dart
index a118edcb0ead40a86926130b66719ea369ed6a0a..59d6270c97b6cf20fd7ce85141fe3c5a88d85e47 100644
--- a/pkg/barback/lib/src/asset_graph.dart
+++ b/pkg/barback/lib/src/asset_graph.dart
@@ -25,9 +25,29 @@ class AssetGraph {
 
   final _phases = <Phase>[];
 
+  /// A stream that emits a [BuildResult] event each time the build is

[Bob Nystrom, 2013/07/10 20:19:44]

remove "event"

[nweiz, 2013/07/10 20:45:09]

Done.

+  /// completed, whether or not it succeeded.
+  ///
+  /// If a programming error in Barback itself occurs, it will be emitted

[Bob Nystrom, 2013/07/10 20:19:44]

Instead of "programming error", maybe "unexpected error".

Lowercase "Barback" here and elsewhere.

[nweiz, 2013/07/10 20:45:09]

Done.
Done.

+  /// through this stream's error channel.
   Stream<BuildResult> get results => _resultsController.stream;
   final _resultsController = new StreamController<BuildResult>.broadcast();
 
+  /// A stream that emits any errors from the asset graph or the transformers.
+  ///
+  /// This emits errors as they're detected. If an error occurs in one part of
+  /// the asset graph, unrelated parts will continue building.
+  ///
+  /// This will not emit programming errors from Barback itself. Those will be
+  /// emitted through the [results] stream's error channel.
+  Stream get errors => _errorsController.stream;

[Bob Nystrom, 2013/07/10 20:19:44]

I think we want a different name for this. Maybe "updates" or "events" or
something? We will very likely also use it for notifying when individual
transforms complete successfully, and for warnings. I'm guessing we'll also add
a way for transforms to log stuff and that will likely go through this.

[nweiz, 2013/07/10 20:45:09]

I'd rather put different types of events in different streams.

In any case, the name now should reflect the events that are emitted now. If we
refactor to change those events in the future, that's when we should change the
name, especially since the API will need to change as well.

[Bob Nystrom, 2013/07/10 21:38:16]

SGTM.

+  final _errorsController = new StreamController.broadcast();
+
+  /// A queue that accumulates errors from a single build.

[Bob Nystrom, 2013/07/10 20:19:44]

How about "The errors that have occurred since the last build completed."

[nweiz, 2013/07/10 20:45:09]

Done, slightly reworded.

+  ///
+  /// This will be empty if no build is occurring.
+  Queue _accumulatedErrors;
+
   /// A future that completes when the currently running build process finishes.
   ///
   /// If no build it in progress, is `null`.
@@ -119,9 +139,9 @@ class AssetGraph {
     _waitForProcess();
   }
 
-  /// Reports a process result with the given error then throws it.
   void reportError(error) {
-    _resultsController.add(new BuildResult(error));
+    _accumulatedErrors.add(error);
+    _errorsController.add(error);
   }
 
   /// Starts the build process asynchronously if there is work to be done.
@@ -133,10 +153,12 @@ class AssetGraph {
   /// to discard it.
   Future _waitForProcess() {
     if (_processDone != null) return _processDone;
+
+    _accumulatedErrors = new Queue();
     return _processDone = _process().then((_) {
       // Report the build completion.
       // TODO(rnystrom): Put some useful data in here.
-      _resultsController.add(new BuildResult());
+      _resultsController.add(new BuildResult(_accumulatedErrors));
     }).catchError((error) {
       // If we get here, it's an unexpected error. Runtime errors like missing
       // assets should be handled earlier. Errors from transformers or other
@@ -149,6 +171,7 @@ class AssetGraph {
       _resultsController.addError(error);
     }).whenComplete(() {
       _processDone = null;
+      _accumulatedErrors = null;
     });
   }
 
@@ -215,14 +238,18 @@ class AssetGraph {
   }
 }
 
-/// Used to report build results back from the asynchronous build process
-/// running in the background.
+/// An event indicating that the asset graph has finished building.
+///
+/// A build can end either in success or failure. If there were no errors during
+/// the build, it's considered to be a success; any errors render it a failure,
+/// although individual assets may still have built successfully.

[Bob Nystrom, 2013/07/10 20:19:44]

I don't think it's meaningful to say a build can succeed or fail. Parts of it
may succeed or fail, but there isn't a single overall output, so I don't think
there's anything special about the case where all transforms happened to
succeed.

[nweiz, 2013/07/10 20:45:09]

I disagree. For users who care about the entire build (e.g. pub deploy) the
distinction between "everything was successful" and "something failed" is
definitely relevant. It's also easy to ignore the [succeeded] field for users
who don't care about it.

It's also telling that our tests care about this notion of success/failure.

 class BuildResult {
-  /// The error that occurred, or `null` if the result is not an error.
-  final error;
+  /// All errors that occurred during the build.
+  final List errors;
 
-  /// `true` if this result is for a successful build.
-  bool get succeeded => error == null;
+  /// `true` if the build succeeded.
+  bool get succeeded => errors.isEmpty;
 
-  BuildResult([this.error]);
+  BuildResult(Iterable errors)
+      : errors = errors.toList();
 }