Skip to content

Overloads with values for HoconRoot getters #144

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

Merged
merged 9 commits into from
Dec 21, 2019

Conversation

IgorFedchenko
Copy link
Contributor

Related to #108

First, existing overloads like obj.GetDouble("a", @default: 5) did not work, because it was expecting underlying GetNode to return null when path was not found, but it was throwing exception (which is generally good thing, but here needed to catch it).

Second - there was no way to specify default value for collection getters (i.e. if I want to get empty list of ids if they are not set - the only options was to handle exception). Added this overloads, and added test for all of them.

@Aaronontheweb
Copy link
Member

Hocon.Tests.HoconTests.GettingArrayFromLiteralsReturnsNull - have a failure on this spec.

@Aaronontheweb
Copy link
Member

(I still don't know why Azure DevOps isn't showing status for PRs in this repository - only shows status for code once it's directly merged into dev)

@IgorFedchenko
Copy link
Contributor Author

Hocon.Tests.HoconTests.GettingArrayFromLiteralsReturnsNull - have a failure on this spec.

Yup, the behavior asserted in this test was incorrect actually, so fixed it too

@Aaronontheweb
Copy link
Member

@IgorFedchenko looks like the CI system is not our friend still

@IgorFedchenko
Copy link
Contributor Author

@IgorFedchenko looks like the CI system is not our friend still

@Aaronontheweb Yup... In Akka.NET also not all steps are completed. Looking at issue here, they still did not resolve this problem.

@Aaronontheweb
Copy link
Member

Hocon.Extensions.Configuration.Tests.ConfigurationSpec.ShouldReloadConfigurationOnFileChange failed - this test seems to be pretty racy (has a high flip rate on Azure DevOps.) I'll take a look at it.

@Aaronontheweb
Copy link
Member

@IgorFedchenko have a merge conflict here

@IgorFedchenko
Copy link
Contributor Author

@IgorFedchenko have a merge conflict here

Since #148 was based in this PR, the changes are already in dev branch, and #148 overwrites some of them.
Anyway, resolved conflicts now, should be ready for merge.

@Aaronontheweb Aaronontheweb merged commit 36d6294 into akkadotnet:dev Dec 21, 2019
@Aaronontheweb
Copy link
Member

Thanks @IgorFedchenko

@IgorFedchenko IgorFedchenko deleted the config-defaults branch January 11, 2020 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants