Skip to content
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

add support for non abbreviated timespan strings #2287

Merged
merged 10 commits into from
Sep 9, 2016
Merged

add support for non abbreviated timespan strings #2287

merged 10 commits into from
Sep 9, 2016

Conversation

mrrd
Copy link
Contributor

@mrrd mrrd commented Sep 2, 2016

This pull request adds support for non abbreviated timespan values in hocon

@Aaronontheweb
Copy link
Member

Lots of test failures....

@Danthar
Copy link
Member

Danthar commented Sep 8, 2016

@mrrd it looks like you broke the parsing of normal values ?

A quick survey reveales that values like: 0.05 s are causing problems now.

Check the remote conf for more samples: https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka.Remote/Configuration/Remote.conf
and the remoteSettings object for a few examples on which are expected to be TimeSpans. https://github.com/akkadotnet/akka.net/blob/dev/src/core/Akka.Remote/RemoteSettings.cs

I think if you fix those scenario's it should probably resolve most, if not all, failing unit tests.

@mrrd
Copy link
Contributor Author

mrrd commented Sep 8, 2016

@Danthar thanks I can see exactly what I have missed a + in the regex

mrrd and others added 9 commits September 8, 2016 12:15
fix the regular expression to account for more than a single digit after
the decimal point
fix the regular expression to account for more than a single digit after
the decimal point

fix test

change brackets to Allman style

change brackets to Allman style

update failing test
@Danthar
Copy link
Member

Danthar commented Sep 9, 2016

@mrrd can you please squash your commits

@Danthar Danthar merged commit 9d278b5 into akkadotnet:dev Sep 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants