Add ICombinedSettingsBuilder.Dump() and ICombinedSettingsBuilder.DumpToSelfLog()

[tsimbalar]

To inspect the currently defined key-value settings and help when debugging settings related issues.

Those extension methods are in the Serilog.Debugging namespace.

Note: this PR sits on top of #10 , so I will rebase it properly once #10 has been merged .

[nblumhardt]

Looks great!

Only a thought - perhaps Inspect() would be a passable alternative name to Dump()? I'm struggling to think of how to reword DumpToSelfLog() as InspectSomethingSomethingSelfLog(), though....

[tsimbalar]

Yep, Inspect() sounds like a better name than Dump()for the overload accepting a Action<IEnumerable<KeyValuePair<string, string>>>.

Maybe PrintToSelfLog() is a better name ? I do like DumpToSelfLog, though, as I think it's a bit clearer that it is a one-off , debugging thing ...

The Dump()name is inspired by LINQPad :)

[nblumhardt]

Not sure if it's a useful suggestion .. but we could always just include Inspect(), and then let the user foreach over the keys and send them to the console/wherever they are conveniently read from. It's nice having the simple to-selflog option, but since it does require finding and setting up the SelfLog, the overall lines-of-code count is similar whether we include the method or not :-)

[tsimbalar]

agreed :)

[tsimbalar]

I've done the change. There is now only .Inspect(Action<IEnumerable<KeyValuePair<string, string>>>).

[nblumhardt]

Another thought, do we need the interface here, or would just using the concrete type throughout eliminate this type of issue?

[tsimbalar]

Do you mean getting rid of the ICombinedSettingsBuilder interface all together ? That would mean making the concrete calss CombinedSettingsBuilder public and sealedtoo, I guess ... I don't have strong opinions on that, just that using interfaces seemed more pure when doing fluent-interface-y stuff :)

[tsimbalar]

and make the constructor internal also.

[nblumhardt]

That was the gist of what I had in mind, but could be pros/cons. Classes and/or ABCs do generally evolve a bit better (it's not a breaking change to add a method, for example), but could be other reasons to prefer an interface here - just an idea :-)

[tsimbalar]

Following your suggestions, I got rid of ICombinedSettingsBuilder in favor of using a concrete class CombinedSettingsBuilder. This should make it easier to extend it without breaking client code down the line.

It's probably paranoia talking, but I have :

• made the class sealed because we don't expect subclassing to be used for extension (at least not for now)
• made the constructor internal because we expect people to interact with an already provided instance, not to build their own
• made the method IEnumerable<KeyValuePair<string, string>> BuildSettings() internal for now ... although I feel a bit strange about having a class called XXXBuilder without a BuildXXX() method 😛

I believe it will still be OK to unlock it later on if we need to :)

[nblumhardt]

Paranoia is welcome here :-)