cs_comments_service
cs_comments_service copied to clipboard
chore: upgrade to ruby 3
- feat: Update testing system
- test: test on ruby 3
- refactor: update to ruby3
Thanks for the pull request, @xitij2000!
As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.
Codecov Report
:exclamation: No coverage uploaded for pull request base (
master@063fd26
). Click here to learn what that means. Patch has no changes to coverable lines.
:exclamation: Current head 712a8bd differs from pull request most recent head afeb30c. Consider uploading reports for the commit afeb30c to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## master #392 +/- ##
=========================================
Coverage ? 96.08%
=========================================
Files ? 58
Lines ? 4522
Branches ? 0
=========================================
Hits ? 4345
Misses ? 177
Partials ? 0
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
@xitij2000 Thank you for the contribution, let me know once this is ready.
@xitij2000 Following our today BTR call, we were disucssing whether discussion (backend service) should run on Ruby 3 or not.
One factor obviously is deadline of this PR, can you by any chance estimae when this would be ready/merged?
Also can you confirm that the alternative is to run the service on a ruby version (2.5.7) ref which already has reached its EOL?
Note: I personally tested this locally but I had to do #395 first and the test was run tutor based using the forum plugin, https://github.com/overhangio/tutor-forum/pull/11 and we are in the process of testing in the acutal production.
It looks like there's also a Ruby 2.7.6 upgrade option that might be easier, but that would also go EOL about 3 months before Palm is released. I think this one slipped under the radar because our Ruby entries in the support windows doc were outdated.
@xitij2000 Following our today BTR call, we were disucssing whether discussion (backend service) should run on Ruby 3 or not.
One factor obviously is deadline of this PR, can you by any chance estimae when this would be ready/merged?
Also can you confirm that the alternative is to run the service on a ruby version (2.5.7) ref which already has reached its EOL?
Note: I personally tested this locally but I had to do #395 first and the test was run tutor based using the forum plugin, overhangio/tutor-forum#11 and we are in the process of testing in the acutal production.
I will try to prioritise this in the upcoming sprint. Initially the aim was just to get tests running on Ruby 3.0 to catch any new failures, and slowly fix things over time, but I can put some of my CC time to getting this running.
@jmbowman I aimed for 3.x for that very reason, that 2.7 was also close to EOL. I will see if I can get all tests passing and if the forum is running without issues on 3.x, and if not I'll see if 2.7 is easier.
@xitij2000 This looks great and it required less changes than I thought it would! I think we'll want to pull in the change from https://github.com/openedx/cs_comments_service/pull/395 before merging, but other than that everything looks good and seems to be working well (I tested it on The Olive demo instance) which is already running code from this branch on ruby 3).
I've got all the tests running and passing on both Ruby 3.0 and 3.1, however with Ruby 3.1 the CI seems to fail due to failed dependencies when building an extension. I might not have time to resolve that, but it is working otherwise.
Thanks a lot for taking care of this @xitij2000! :+1:
- [x] I tested this: I verified that the forums are working correctly on the Olive demo instance.
- [x] I read through the code
- [ ] I checked for accessibility issues (N/A)
- [ ] Includes documentation (N/A)
@asadazam93, is this something you could look at in the next couple of days? We want to have it in as part of the Olive release, since Ruby < 3 is out of support (and has been for a while).
@xitij2000 Is this a breaking change?
@xitij2000 Is this a breaking change?
It should not be. This PR is tested against both the current version of Ruby (2.5.7) and on Ruby 3.0.x. However, the idea is to eventually start using Ruby 3.0, so it sets the stage for that.
Hi folks, Would it be possible for this to be merged today or by tomorrow at least. Since then it need to be backported to olive and then tested again with the recent commits to olive...etc. Thus the sooner this is merged the less urguent things would need to be done for the olive relaese.
I appercaite your understaning and happy to help if anything is needed with this.
@xitij2000 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.