-
Notifications
You must be signed in to change notification settings - Fork 218
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
[DEPR] xblock.fragment and direct id_generator parameters in xblock.runtime.Runtime #680
Conversation
irtazaakram
commented
Oct 19, 2023
- Resolves [DEPR] xblock.fragment and direct id_generator parameters in xblock.runtime.Runtime public-engineering#15
@irtazaakram it looks like there are conflicts and failing tests, can you put the PR in Draft until it's ready for review? Or would you like a review with the tests as is? |
Hi @feanil, I've put this PR in Draft. It requires openedx/xblock-sdk#323 this to be merged first. |
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 think a new version of this repo will need to be release before we can update xblock-sdk to be using that new version of the repo.
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.
Changes look good to me. @farhan can you also do a review of this change?
Looks like there is still a couple of tests that need to be fixed as well. |
Hi @feanil , These tests are failing because of
We would need to release Thanks, |
@irtazaakram really good point, the XBlock repo should not depend on the xblock-sdk repo, since that will cause a circular dependency that will make it hard to do incremental releases. I've made #688 to resolve the issue so that this repo does not depend on xblock-sdk. Once that's merged, we should be able to rebase this PR and land it. |
@iamsobanjaved is this ready for review? |
@irtazaakram 2 request changes rest seems good, nice work ⭐ Please rebase the PR |
upgrade xblock==2.0 References: - openedx/public-engineering#15 - openedx/XBlock#680
* Update the import statement for xblock-utils The `xblock-utils` library has been deprecated as a separate package; the `utils` library has been moved into the `XBlock` and should now be imported from `xblock.utils` instead. (openedx/XBlock#675) * Upgrade to XBlock 2 Remove the use of deprecated `xblock.fragment` and direct id_generator parameters. 9openedx/XBlock#680) * Add Python 3.11 to test matrix Fixes: citynetwork#38
* Update the import statement for xblock-utils The `xblock-utils` library has been deprecated as a separate package; the `utils` library has been moved into the `XBlock` and should now be imported from `xblock.utils` instead. (openedx/XBlock#675) * Upgrade to XBlock 2 Remove the use of deprecated `xblock.fragment` and direct id_generator parameters. 9openedx/XBlock#680) * Add Python 3.11 to test matrix; Drop Python 3.8 from test matrix * Add a version compatibility matrix to the README Fixes: citynetwork#38