Finalize design for phases API

[kevmoo]

import 'dart:async';

main() async {
  await build([[simplePhase]]);
  await build([parallelSinglePhase]);
  await build(multiPhase);
}

final simplePhase = new Builder(new InputSet('foo', const ['lib/**/*.dart']));

final parallelSinglePhase = [
  new Builder(new InputSet('foo', const ['lib/**/*.dart'])),
  new Builder(new InputSet('bar', const ['lib/**/*.dart'])),
  new Builder(new InputSet('baz', const ['lib/**/*.dart']))
];

final multiPhase = [parallelSinglePhase, [simplePhase]];

// ***** implementation

Future build(List<List<Builder>> input) async {}

class Builder  {
  const Builder(InputSet inputs);

  Future build(/*BuildStep*/ bs) async {}
}

class InputSet {
  const InputSet(String packageName, List<String> globs);

  InputSet.currentPackage(Iterable<String> globs);

  // package
  // glob+
}

[jakemac53]

So, thinking about this a bit more it does seem a bit unfortunate to have to make all Builder classes be explicitly aware of InputSets. Even if its just forwarding to the super constructor, its unfortunate. It also means potentially creating extra instances of Builders just to cover multiple input sets....

[jakemac53]

What if we used the builder pattern, and did something like this?

// USAGE

class FakeBuilder implements Builder {
  Future build(_) async => throw new UnimplementedError();

  List<AssetId> declareOutputs(AssetId input) => throw new UnimplementedError();
}

main() async {
  var builder = new FakeBuilder();
  var fooLibInputs = new InputSet('foo', const ['lib/**/*.dart']);
  var barLibInputs = new InputSet('bar', const ['lib/**/*.dart']);
  var bazLibInputs = new InputSet('baz', const ['lib/**/*.dart']);

  // Shorthand for the simple case.
  await build(new PhaseGroup.singleAction(builder, fooLibInputs));

  // Alternative version of the simple case.
  var singlePhase = new Phase()..addAction(builder, fooLibInputs);
  await build(new PhaseGroup()..addPhase(singlePhase));

  // A Phase with multiple concurrent actions 
  var parallelPhase = new Phase()
      ..addAction(builder, fooLibInputs)
      ..addAction(builder, barLibInputs)
      ..addAction(builder, bazLibInputs);
  await build(new PhaseGroup()..addPhase(parallelPhase));

  // Multiple phases that run sequentially
  await build(new PhaseGroup()..addPhase(singlePhase)..addPhase(parallelPhase));
}

// IMPLEMENTATION

class BuildAction {
  final Builder builder;
  final InputSet inputSet;

  BuildAction(this.builder, this.inputSet);
}

class Phase {
  final _actions = <BuildAction>[];
  List<BuildAction> get actions => new List.unmodifiable(_actions);

  addAction(Builder builder, InputSet inputSet) {
    _actions.add(new BuildAction(builder, inputSet));
  }
}

class PhaseGroup {
  final _phases = <Phase>[];
  List<List<BuildAction>> get buildActions =>
      new List.unmodifiable(_phases.map((phase) => phase.actions));

  PhaseGroup();

  // Could add additional constructors which take lists of phases, etc later on.
  factory PhaseGroup.singleAction(Builder builder, InputSet inputSet) =>
      new PhaseGroup().newPhase().addAction(builder, inputSet);

  Phase newPhase() {
    var phase = new Phase();
    _phases.add(phase);
    return phase;
  }

  void addPhase(Phase phase) {
    _phases.add(phase);
  }

  String toString() {
    var buffer = new StringBuffer();
    for (int i = 0; i < _phases.length; i++) {
      buffer.writeln('Phase $i:');
      for (var action in _phases[i].actions) {
        buffer
          ..writeln('  Action:')
          ..writeln('    Builder: ${action.builder}')
          ..writeln('    InputSet:${action.inputSet}');
      }
    }

    return buffer.toString();
  }
}

It is a bit more boilerplate but I think its also more readable. This also has the advantage of allowing you to pass a PhaseGroup around so libraries can easily provide helpers for setting up the phases for their builders.

[jakemac53]

cc @nex3 @munificent what do you think about the last option I posted?

[jakemac53]

Another option would be to make the top level build function take a List<List<BuildAction>> instead of a PhaseGroup. Then PhaseGroup becomes a totally optional thing, and you can use the nested lists if you desire.

[nex3]

The example code seems pretty reasonable to me.

[jakemac53]

@nex3 do you mean the first one @kevmoo pasted or the followup one I pasted?

[nex3]

Your follow-up.