Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: upgrade dependencies and ruby minor version #416

Closed

Conversation

ghassanmas
Copy link
Member

@ghassanmas ghassanmas commented Aug 15, 2023

This change does the following (for Gemfile3)

  • It update the dependencies via BUNDLE_GEMFILE=Gemfile3 bundle update

    • nokogiri manually bump version to latest
  • It runs the tests on ruby 3.0.6 iatest release version

  • This pull request is not tested yet.

  • After merge, backport to pam

@openedx-webhooks
Copy link

openedx-webhooks commented Aug 15, 2023

Thanks for the pull request, @ghassanmas! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 15, 2023
@ghassanmas ghassanmas force-pushed the upgrade-depns-and-ruby-minor branch 5 times, most recently from 18e933a to 925d883 Compare August 15, 2023 08:34
@ghassanmas ghassanmas marked this pull request as draft August 15, 2023 08:40
@ghassanmas ghassanmas force-pushed the upgrade-depns-and-ruby-minor branch 7 times, most recently from b0d79bb to 1ee2b58 Compare August 15, 2023 11:00
@ghassanmas ghassanmas marked this pull request as ready for review August 15, 2023 11:01
@ghassanmas ghassanmas force-pushed the upgrade-depns-and-ruby-minor branch 2 times, most recently from 5856921 to 5e0f292 Compare August 15, 2023 11:05
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (81e11c7) 96.14% compared to head (fb408bc) 96.14%.

❗ Current head fb408bc differs from pull request most recent head e55948a. Consider uploading reports for the commit e55948a to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #416   +/-   ##
=======================================
  Coverage   96.14%   96.14%           
=======================================
  Files          58       58           
  Lines        4562     4562           
=======================================
  Hits         4386     4386           
  Misses        176      176           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@itsjeyd
Copy link

itsjeyd commented Aug 15, 2023

Hi @ghassanmas, thanks for the contribution! Let us know when the changes are ready for review please.

@itsjeyd itsjeyd added core contributor PR author is a Core Contributor (who may or may not have write access to this repo). waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Aug 15, 2023
@itsjeyd
Copy link

itsjeyd commented Sep 13, 2023

Hey @ghassanmas, just checking in to see if you're still planning to continue working on this PR?

@ghassanmas
Copy link
Member Author

@itsjeyd Apologies for the late reply, I can confirm this has been tested, and to my end it's ready for review

@itsjeyd
Copy link

itsjeyd commented Sep 21, 2023

@ghassanmas No problem, and thanks for the update!

It looks like one of the required checks didn't run for some reason. Do you have any idea why? If not, would you mind rebasing your changes and pushing them again to trigger another build?

@ghassanmas
Copy link
Member Author

ghassanmas commented Sep 21, 2023

@itsjeyd I am not really sure, but I can retriigger the test... I will do that now, already tests are running now

@ghassanmas ghassanmas force-pushed the upgrade-depns-and-ruby-minor branch 2 times, most recently from 4258cb8 to c207f72 Compare September 21, 2023 13:41
@itsjeyd itsjeyd added needs test run Author's first PR to this repository, awaiting test authorization from Axim and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Sep 22, 2023
@itsjeyd
Copy link

itsjeyd commented Sep 22, 2023

@ghassanmas Hm ... 🤔 Looks like that didn't do the trick.

@e0d Do you have any idea what might be happening here?

@itsjeyd
Copy link

itsjeyd commented Oct 10, 2023

Hey @ghassanmas, looks like master was updated with some conflicting changes. Maybe you could try rebasing again and see if that helps with getting remaining checks to run?

@ghassanmas ghassanmas force-pushed the upgrade-depns-and-ruby-minor branch 2 times, most recently from fb33e43 to d9d9fda Compare October 11, 2023 17:17
@ghassanmas ghassanmas force-pushed the upgrade-depns-and-ruby-minor branch from d9d9fda to e55948a Compare October 11, 2023 20:27
@ghassanmas
Copy link
Member Author

@itsjeyd this is no longer needed, hecne #418

@ghassanmas ghassanmas closed this Oct 11, 2023
@openedx-webhooks
Copy link

@ghassanmas Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

1 similar comment
@openedx-webhooks
Copy link

@ghassanmas Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@e0d e0d removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants