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 Duration WellKnownType JSON Support #102

Merged
merged 2 commits into from
Oct 29, 2020
Merged

Conversation

nickthegroot
Copy link
Contributor

@nickthegroot nickthegroot commented Oct 28, 2020

Another box checked in #72

@nickthegroot nickthegroot requested a review from garyp October 28, 2020 21:19
Copy link
Collaborator

@garyp garyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this 🙇 A few minor comments. Looks good to merge whenever you're done with them.

runtime/src/commonMain/kotlin/pbandk/internal/TimeUtil.kt Outdated Show resolved Hide resolved
var seconds = dur.seconds
var nanos = dur.nanos

return buildString {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 A slight performance optimization: pre-allocate the largest length this string can be (which I think is 25 characters) by using buildString(25) {. That way the builder doesn't have to do unnecessary string copies during serialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's 24: -315576000000.999999999s

runtime/src/commonMain/kotlin/pbandk/internal/TimeUtil.kt Outdated Show resolved Hide resolved
runtime/src/commonMain/kotlin/pbandk/internal/TimeUtil.kt Outdated Show resolved Hide resolved
runtime/src/commonMain/kotlin/pbandk/internal/TimeUtil.kt Outdated Show resolved Hide resolved
@nickthegroot nickthegroot merged commit 053a13e into master Oct 29, 2020
@nickthegroot nickthegroot deleted the json/durations branch October 29, 2020 22:03
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