Do we have this tested directly or indirectly somewhere?

This is tested indirectly through the Aggregate_out_to_time_series_collection_on_secondary_should_work test and the AggregateToCollection time series tests. Without this, the simplification of the out stage will just remove the timeseries options resulting in the time series tests failing.

Looks like we just need the collection namespace for rendering.
Maybe we can just mock the collection, and then the test would not be dependent on server?

Same comment about mocking collection.

Not sure that we need the secondary feature.

We should either merge overloads (add timeSeriesOptions = null parameter) or forbid null here.
I personally would prefer having less overloads for simplicity.

Optional parameters are problematic and I think we should avoid them in our public API. We can use them internally where it makes sense, but not in our public API.

Reason 1: The optional value is inserted into the calling code at compile-time for the calling code. So if you change your default value, but don't recompile the calling code, you'll still use the same value.

Reason 2: Even more insidious is that the C# compiler doesn't enforce consistency among default values. The following is perfectly valid:

var oops1 = new EvilClass();
var oops2 = oops1 as IOops;
Console.WriteLine(oops1.GetValue());
Console.WriteLine(oops2.GetValue());
interface IOops
{
 int GetValue(int x = 42);
}
class EvilClass : IOops
{
 public int GetValue(int x = 99) => x;
}

The output is:

99
42

I see that we are already doing this elsewhere in our Fluent Aggregate API. Sigh. I guess we can use TimeSeriesOptions timeSeriesOptions = null here to reduce the number of overloads.

Alphabetical order (field and property).

time-series => time series

And add period at end of the param description.

Optional parameters are problematic and I think we should avoid them in our public API. We can use them internally where it makes sense, but not in our public API.

Reason 1: The optional value is inserted into the calling code at compile-time for the calling code. So if you change your default value, but don't recompile the calling code, you'll still use the same value.

Reason 2: Even more insidious is that the C# compiler doesn't enforce consistency among default values. The following is perfectly valid:

var oops1 = new EvilClass();
var oops2 = oops1 as IOops;
Console.WriteLine(oops1.GetValue());
Console.WriteLine(oops2.GetValue());
interface IOops
{
 int GetValue(int x = 42);
}
class EvilClass : IOops
{
 public int GetValue(int x = 99) => x;
}

The output is:

99
42

Period at end of sentence.

I see that we are already doing this elsewhere in our Fluent Aggregate API. Sigh. I guess we can use TimeSeriesOptions timeSeriesOptions = null here to reduce the number of overloads.