-
Notifications
You must be signed in to change notification settings - Fork 40
Fix Hocon to work with Akka.NET #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - going to need to bump this into a new version since we're changing the name of the Parser
class.
@@ -577,6 +577,28 @@ public void ShouldSerializeFallbackValues() | |||
|
|||
} | |||
|
|||
[Fact] | |||
public void WithFallback_ShouldNotChangeOriginalConfig() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good
/// <exception cref="T:System.ArgumentNullException">"The root value cannot be null."</exception> | ||
public Config(HoconRoot root) : base(root?.Value, root?.Substitutions ?? Enumerable.Empty<HoconSubstitution>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much simpler - I like it
} | ||
} | ||
|
||
return _mergedValueCache; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good idea - caching this value for future computations.
/// </summary> | ||
/// <param name="fallback">The configuration to use as a secondary source.</param> | ||
/// <returns>The current configuration configured with the specified fallback.</returns> | ||
/// <exception cref="ArgumentException">Config can not have itself as fallback.</exception> | ||
public virtual Config WithFallback(Config fallback) | ||
{ | ||
if (fallback == this) | ||
if (ReferenceEquals(fallback, this)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that's the right call - since we might overload ==
return Copy(Fallback.SafeWithFallback(fallback)); | ||
|
||
if (IsEmpty) | ||
return fallback; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense
Related to akkadotnet/akka.net#4128
Parser
class name toHoconParser