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

try harder to avoid long overflow in Time#+ (fixes #1779) #4646

Merged
merged 4 commits into from
Jun 5, 2017

Conversation

MSNexploder
Copy link
Contributor

No description provided.

@kares
Copy link
Member

kares commented Jun 5, 2017

... very nice Stefan, could we have a test or spec please (as this is easy to break at some point) ?

@MSNexploder
Copy link
Contributor Author

Added a small test (and reenabled some existing time related regression tests which pass) but i'm not sure if this is the best way to test.

@kares
Copy link
Member

kares commented Jun 5, 2017

@MSNexploder thanks, its not bad (you can leave it in) but does not actually test for the reported issue,
a new test which would test smt like :

t = Time.local(2000, 1, 1) + (300 * 366 * 24 * 60 * 60)
assert_equal 2300, t.year

... would probably do, what do you think?

@MSNexploder
Copy link
Contributor Author

@kares That was too easy 😎 Was focusing way to much on the underlying problem.

@kares
Copy link
Member

kares commented Jun 5, 2017

cool thanks a lot ... @headius could you double-check this is good to go for @MSNexploder, please?

@headius headius merged commit dad5ac5 into jruby:master Jun 5, 2017
@headius
Copy link
Member

headius commented Jun 5, 2017

Looks good, thank you!

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.

None yet

3 participants