Drop support for EOL Ruby versions (2.5 and 2.6)#2538
Drop support for EOL Ruby versions (2.5 and 2.6)#2538Zeragamba merged 8 commits intofaker-ruby:masterfrom
2.5 and 2.6)#2538Conversation
thdaraujo
left a comment
thdaraujo
left a comment
There was a problem hiding this comment.
nice, thanks for working on this!
I left a couple of questions regarding the tests we probably still want to keep.
| assert_raise Date::Error do | ||
| ::Date.new(@today.year - @min, @today.month, @today.day) | ||
| end | ||
|
|
||
| assert_raise Date::Error do | ||
| ::Date.new(@today.year - @max, @today.month, @today.day) | ||
| end |
There was a problem hiding this comment.
Yes, we want to remove the tests for version < 2.7 and we don't need the version checks anymore.
But we still want to keep the tests that check that the error raised is a Date::Error, right? I don't think you need to remove them.
|
what do you think of also removing Ruby versions older than 2.7 from the test matrix? Here: faker/.github/workflows/ruby.yml Line 38 in 4ffc714 |
…ap_year and test_invalid_south_african_id_number
|
@thdaraujo I removed the ruby versions older than 2.7 from the test matrix and fixed the 2 errors from my previous pull request. Thank you for your help |
nick-mendez
left a comment
nick-mendez
left a comment
There was a problem hiding this comment.
I revised the two test files to include the errors without the version check. I also removed the older ruby versions ( < 2.7) from the test matrix
stefannibrasil
left a comment
stefannibrasil
left a comment
There was a problem hiding this comment.
Thank you, @nickmendezFlatiron this is great! I believe there is a duplicated block to be removed. Could you remove it and then we can approve this? Thanks!
|
@nickmendezFlatiron if you run |
…rected offenses with rubocop
nick-mendez
left a comment
nick-mendez
left a comment
There was a problem hiding this comment.
I have made the suggested changes using rubocop and removed the duplicate code from the test file. Thank you for your help
stefannibrasil
left a comment
stefannibrasil
left a comment
There was a problem hiding this comment.
Awesome, thank you!
thdaraujo
changed the title
2.5 and 2.6)
4 participants
Summary
I Completed the 3 tasks in Issue #2529
Other Information