Skip to content

Conversation

@irtazaakram
Copy link
Member

@irtazaakram irtazaakram marked this pull request as draft October 19, 2023 06:12
@irtazaakram irtazaakram marked this pull request as ready for review October 19, 2023 07:11
@feanil
Copy link
Contributor

feanil commented Oct 23, 2023

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

@irtazaakram irtazaakram marked this pull request as draft October 24, 2023 06:41
@irtazaakram
Copy link
Member Author

Hi @feanil, I've put this PR in Draft. It requires openedx/xblock-sdk#323 this to be merged first.

Copy link
Contributor

@feanil feanil left a 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.

@irtazaakram irtazaakram requested a review from feanil October 25, 2023 11:52
@irtazaakram irtazaakram marked this pull request as ready for review October 31, 2023 05:20
Copy link
Contributor

@feanil feanil left a 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?

@feanil
Copy link
Contributor

feanil commented Oct 31, 2023

Looks like there is still a couple of tests that need to be fixed as well.

@irtazaakram
Copy link
Member Author

Hi @feanil ,

These tests are failing because of xblock-sdk as mentioned here

Hi @feanil,
I've put this PR in Draft. It requires openedx/xblock-sdk#323 this to be merged first.

#680 (review)

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.

We would need to release 0.7.1 version for xblock-sdk to clear these tests.

Thanks,

@feanil feanil requested review from farhan and salman2013 November 3, 2023 13:16
@feanil
Copy link
Contributor

feanil commented Nov 7, 2023

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

@feanil
Copy link
Contributor

feanil commented Jan 11, 2024

@iamsobanjaved is this ready for review?

@farhan
Copy link
Contributor

farhan commented Feb 7, 2024

@irtazaakram 2 request changes rest seems good, nice work ⭐

Please rebase the PR

@irtazaakram irtazaakram merged commit 2dcc942 into master Feb 28, 2024
@irtazaakram irtazaakram deleted the PE#15 branch February 28, 2024 10:39
irtazaakram added a commit to openedx/openedx-platform that referenced this pull request Apr 8, 2024
mrtmm pushed a commit to mrtmm/markdown-xblock that referenced this pull request Aug 2, 2024
* 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: cleura#38
mrtmm pushed a commit to mrtmm/markdown-xblock that referenced this pull request Aug 2, 2024
* 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: cleura#38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[DEPR] xblock.fragment and direct id_generator parameters in xblock.runtime.Runtime

4 participants