Skip to content

Simplified toString() for duration #490

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 1 commit into from
Apr 23, 2018

Conversation

lutovich
Copy link
Contributor

Previously it normalized duration in order to expose a more human-friendly string. For example, 7 days would become a week, 12 months would become a year. It seems nice but might create a false impression that it is possible to retrieve weeks and years from an IsoDuration object. However, it only supports months, days, seconds and nanoseconds. Also, other drivers do not normalize durations.

This PR makes IsoDuration#toString() simply concatenate all values into a string as is. Resulting string will only contain months, days, seconds and nanoseconds. Zero values are not filtered out. So returned ISO strings represent exactly same durations but in different format.

Previously it normalized duration in order to expose a more
human-friendly string. For example, 7 days would become a week,
12 months would become a year. It seems nice but might create a
false impression that it is possible to retrieve weeks and years
from an `IsoDuration` object. However, it only supports months,
days, seconds and nanoseconds. Also, other drivers do not normalize
durations.

This commit makes `IsoDuration#toString()` simply concatenate all
values into a string as is. Resulting string will only contain months,
days, seconds and nanoseconds. Zero values are not filtered out. So
returned ISO strings represent exactly same durations but in different
format.
@lutovich lutovich requested review from zhenlineo and ali-ince April 20, 2018 21:32
@zhenlineo zhenlineo merged commit f6c4e9b into neo4j:1.6 Apr 23, 2018
@lutovich lutovich deleted the 1.6-duration-toString branch April 23, 2018 09:33
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