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

fixed minor overflow bugs in Time and Duration #58

Closed

Conversation

subject-name-here
Copy link
Contributor

No description provided.

@dirk-thomas
Copy link
Member

dirk-thomas commented Jul 20, 2017

@subject-name-here Thank you for pointing out these issue and proposing a fix. The new tests have been very helpful. The actual code changes to make the tests pass seem a bit "too complicated" to me. Since I couldn't describe why and how it should look like instead I tried to come up with a simpler patch which passes your tests. Please see the proposed changes in #61. Do you agree that #61 also addresses the issue sufficiently and is slightly easier to read / understand?

@dirk-thomas
Copy link
Member

I will close this in favor of #61. It would be great if you could still comment here or on #61 about the modified patch. Thanks!

@subject-name-here
Copy link
Contributor Author

@dirk-thomas Yes, I agree. Your patch is much easier to read and it covers the issue.
But I have a little question. It's not very important, but it can cause misunderstanding.

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.

2 participants