-
Notifications
You must be signed in to change notification settings - Fork 66
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
Add support for django 4.2 #107
Conversation
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.
@irtazaakram, Django 4 is never used in these tests. It seems that this command installs django==3.2.11
directly from the xblock-sdk
requirements.
Hi @Agrendalath, Thank you for the review. I've made some changes in tox that should install Django 4.2 in test env. Also I've updated package versions in requirements. Thanks, |
@irtazaakram, I proposed an alternative approach at #108. Would you like to cherry-pick 8d3040b into your PR so we can merge it here? |
Hi @Agrendalath, I've updated code as per your approach. Thanks, |
@irtazaakram, please revert the last commit and cherry-pick 8d3040b to ensure all changes are included. |
91204ad
to
d9edbc6
Compare
@Agrendalath, I've cherry-picked 8d3040b into this branch. Please review. |
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 tested this: checked that the CI is passing
- I read through the code
- I checked for accessibility issues: n/a
- Includes documentation: n/a
- I made sure any change in configuration variables is reflected in the corresponding client's
configuration-secure
repository: n/a
Hi,
This PR is related to ongoing effort to upgrade openedx to Django 4.2
edx/upgrades#368
Please can we have a new release after this PR is merged.
Thanks,