-
Notifications
You must be signed in to change notification settings - Fork 20
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 to ruby 3.3 closes #33 #34
Conversation
@ghassanmas Hi, the upstream PR has been merged. What's the status of this PR? |
@DawoudSheraz if we merge it as it is, but then we need to use open-release/redwood.master as opposite to the default open-release/redwood.1 (redwood.1 doesn't have ruby update), or we wait for redwood.2. |
If we think this update to be important, we can draft v18.1.0. We don't need to wait for redwood.2. |
tutorforum/plugin.py
Outdated
@@ -23,7 +23,8 @@ | |||
"PORT": "4567", | |||
"API_KEY": "forumapikey", | |||
"REPOSITORY": "https://github.com/openedx/cs_comments_service.git", | |||
"REPOSITORY_VERSION": "{{ OPENEDX_COMMON_VERSION }}", | |||
# TEMP: after redwood.2 release replace with {{ OPENEDX_COMMON_VERSION }} | |||
"REPOSITORY_VERSION": "open-release/redwood.master", | |||
}, | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DawoudSheraz I have changed the default for FORUM_REPOSITORY_VERSION
to open-release/redwood.master
. if you okay with that it's ready to be reviewed/mrged, I have test it with tutor 18 already
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, you dont need to change this value. This will be filled automatically based on tutor version (master or nightly).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant to change so if make new release upon merge, consumers will be able to utilzie the new update, otherwise we wait for redwood.2... in case it's important, otherwise I would say let's wait for redwood.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I get it. Since upstream change is important for this PR, we can wait for redwood.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ghassanmas redwood.2 tagged tutor release overhangio/tutor#1110 is out, this PR should be good to go now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ghassanmas Hi, catching up on ☝🏽
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I was on holiday most of the past week, I will take a look into this and #43 and then create a draft a reelase PR, today...
047fe6a
to
6634df7
Compare
Work on progress, changes here will depends on upstream PR:
- openedx/cs_comments_service#429Since it was merged after redwood, to be updated before redwood 2 or v18.0.1