cs_comments_service icon indicating copy to clipboard operation
cs_comments_service copied to clipboard

chore: upgrade to ruby 3

Open xitij2000 opened this issue 2 years ago • 6 comments

  • feat: Update testing system
  • test: test on ruby 3
  • refactor: update to ruby3

xitij2000 avatar Sep 21 '22 06:09 xitij2000

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.

openedx-webhooks avatar Sep 21 '22 06:09 openedx-webhooks

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.

codecov[bot] avatar Sep 21 '22 06:09 codecov[bot]

@xitij2000 Thank you for the contribution, let me know once this is ready.

natabene avatar Sep 21 '22 15:09 natabene

@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.

ghassanmas avatar Oct 24 '22 16:10 ghassanmas

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.

jmbowman avatar Oct 24 '22 16:10 jmbowman

@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 avatar Oct 26 '22 11:10 xitij2000

@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).

mtyaka avatar Nov 09 '22 08:11 mtyaka

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.

xitij2000 avatar Nov 09 '22 11:11 xitij2000

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)

mtyaka avatar Nov 14 '22 14:11 mtyaka

@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).

arbrandes avatar Dec 05 '22 15:12 arbrandes

@xitij2000 Is this a breaking change?

asadazam93 avatar Dec 05 '22 15:12 asadazam93

@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.

xitij2000 avatar Dec 06 '22 12:12 xitij2000

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.

ghassanmas avatar Dec 08 '22 10:12 ghassanmas

@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.

openedx-webhooks avatar Dec 09 '22 11:12 openedx-webhooks