Skip to content

[#2867] modified Date#>> to take calendar reforms under consideration#3091

Closed
ransoca wants to merge 6 commits intojruby:masterfrom
ransoca:2867_date_next_year_difference
Closed

[#2867] modified Date#>> to take calendar reforms under consideration#3091
ransoca wants to merge 6 commits intojruby:masterfrom
ransoca:2867_date_next_year_difference

Conversation

@ransoca
Copy link

@ransoca ransoca commented Jun 29, 2015

…ation

- added tests for next month with calendar reform
- uses solution provided in jruby#1769 comments
- also fixes issues jruby#1769
@kares
Copy link
Member

kares commented Jun 29, 2015

@raeoks would you mind trying to target jruby-1_7 so that this is fixed in JRuby 1.7.x as well?
... don't think test should be added under test/mri as those might conflict when the MRI suite is merged

@eregon since its mostly your work-around ... any thoughts on getting those in?

@eregon
Copy link
Member

eregon commented Jun 29, 2015

These tests would be useful in RubySpec. You can just do a separate commit touching spec/ruby.

Well, if this behavior must be supported, I'd say there is not much choice.

@ransoca
Copy link
Author

ransoca commented Jun 30, 2015

I'll move test cases from test/mri/date/test_date_arith.rb to spec/ruby/library/date's add_month_spec.rb and next_year_spec.rb files.
Also I'll be start working on creating a new pull request to fix same issue on jruby-1_7.

@headius
Copy link
Member

headius commented Jul 7, 2015

The changes look ok to me but I defer to @eregon on when and whether this should merge.

@eregon
Copy link
Member

eregon commented Jul 8, 2015

@raeoks Could you move the test cases to specs? Or should I do it?

@ransoca
Copy link
Author

ransoca commented Jul 8, 2015

@eregon I'll do it and doing it right now.

@ransoca
Copy link
Author

ransoca commented Jul 20, 2015

@eregon Should I create a new pull request resolving #2867 for Jruby 1.7?

@eregon
Copy link
Member

eregon commented Jul 20, 2015

I guess yes but better ask @kares.
@raeoks Is this PR ready to be merged?

@kares
Copy link
Member

kares commented Jul 21, 2015

optimally, there would be only one PR targeting jruby-1_7 (branch is periodically merged to master) ...

@eregon
Copy link
Member

eregon commented Jul 21, 2015

I applied the commits to 1.7, 1.9-mode (1.8 Date is just so different and should not be affected the same way by this bug): 40e9140 and ec80a32.

I guess the merge in this case would not work so well (date.rb moved quite a bit), so I'd rather merge #3157 in master instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants