Skip to content

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

Merged
headius merged 4 commits intojruby:masterfrom
MSNexploder:time_add
Jun 5, 2017
Merged

try harder to avoid long overflow in Time#+ (fixes #1779)#4646
headius merged 4 commits intojruby:masterfrom
MSNexploder:time_add

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.

3 participants