Adds xblock-utils repository code into this repository #669

farhan · 2023-09-07T12:42:26Z

Adds xblock-utils repository code into this repository

Relevant ticket: #675

Implementation details:
I have added xblock-utils code in the xblock/utils package and added their test cases in xblock/test/utils package

I didn't move the bok-choy integration test cases into the repo because the library has been deprecated.

Child PR to move docs: #672

feanil

@farhan rather than bringing the xbolckutils module directly, I think we want to move all the relevant code into a xblock.utils module. This keeps the code in the xblocks repository cleaner in the long run but will involve a bit more complexity to the move. Can you try to update this so that the code lives under the existing xblocks structure?

farhan · 2023-09-11T14:41:20Z

@farhan rather than bringing the xbolckutils module directly, I think we want to move all the relevant code into a xblock.utils module. This keeps the code in the xblocks repository cleaner in the long run but will involve a bit more complexity to the move. Can you try to update this so that the code lives under the existing xblocks structure?

@feanil Yes we can

I kept it separate for following 2 pros:

All the repos who are using xblock-utils don't need to update the imports
xblock-utils will stay as a separate layer

But if we want to keep it intact with the xblocks repo, it should reside inside

Let me refactor it.

1 more thing I am not going to add the bok-choy integration tests as we have deprecated the package

feanil · 2023-09-11T17:58:17Z

1 more thing I am not going to add the bok-choy integration tests as we have deprecated the package

That makes sense to me.

feanil

@farhan looks pretty good, there are a few more things I need to check before I can finish the review but so far this looks good. Can you link this PR to the corresponding PR in the xblock-utils repo?

xblock/test/test_plugin.py

xblock/utils/__init__.py

xblock/utils/publish_event.py

xblock/utils/resources.py

xblock/utils/settings.py

xblock/utils/studio_editable.py

feanil

A couple more comments and then I think this will be ready for review by Dave/Kyle.

.github/workflows/ci.yml

setup.py

xblock/test/test_plugin.py

xblock/utils/resources.py

feanil

@farhan Just one more note about the version.

xblock/__init__.py

requirements/base.in

ormsbee

I did not review all the files in the utils and test/utils packages because I assume they're coming straight over from xblockutils (except for the bok choy stuff that was dropped).

I'm also assuming that some manual testing is being done to make sure that things still work when we have a mix of XBlocks where some XBlocks are using xblock.utils and some are using xblockutils and we're installing both libraries at the same time–since this mode is likely to be very common for a while.

Otherwise, it looks good to me.

farhan · 2023-09-21T06:03:54Z

Thanks for review and sharing thoughts @ormsbee

I did not review all the files in the utils and test/utils packages because I assume they're coming straight over from xblockutils (except for the bok choy stuff that was dropped).

Yes they are coming straight from repo, no need to review all

I'm also assuming that some manual testing is being done to make sure that things still work when we have a mix of XBlocks where some XBlocks are using xblock.utils and some are using xblockutils and we're installing both libraries at the same time–since this mode is likely to be very common for a while.

Otherwise, it looks good to me.

Yes, some manual testing is done on DoneXBlock and repo is working fine with different pip package cases:

Screen Shots

feanil

@farhan I think you need to add a changelog entry to summarize that this is a move of code from the xblock-utils repository and then I think this is good to merge and release.

farhan · 2023-09-25T11:39:44Z

@farhan I think you need to add a changelog entry to summarize that this is a move of code from the xblock-utils repository and then I think this is good to merge and release.

@feanil I have added the changelog and merged the docs PR into this PR as well.
Please review the change logs, are we good to merge it and release it now?

change logs file

feanil

One small suggestion to the CHANGELOG and then I think this is good to merge and release.

CHANGELOG.rst

feanil

👍🏾

farhan force-pushed the farhan/merge-xblock-utils branch from d4776c4 to ec610f2 Compare September 7, 2023 12:43

farhan changed the title ~~Adds xblock-utils repository code into this repository~~ [WIP] Adds xblock-utils repository code into this repository Sep 7, 2023

farhan force-pushed the farhan/merge-xblock-utils branch 2 times, most recently from 3763d1f to c616622 Compare September 8, 2023 13:37

farhan requested a review from feanil September 8, 2023 14:26

farhan force-pushed the farhan/merge-xblock-utils branch 4 times, most recently from bda7660 to 2611c6e Compare September 11, 2023 14:25

feanil requested changes Sep 11, 2023

View reviewed changes

farhan force-pushed the farhan/merge-xblock-utils branch from 2611c6e to a5e51c9 Compare September 11, 2023 14:31

farhan force-pushed the farhan/merge-xblock-utils branch 2 times, most recently from 87b946e to 76eaea3 Compare September 12, 2023 07:50

farhan closed this Sep 12, 2023

farhan reopened this Sep 12, 2023

farhan force-pushed the farhan/merge-xblock-utils branch from 4a49256 to 617d313 Compare September 12, 2023 12:46

farhan requested a review from feanil September 12, 2023 13:33

farhan changed the title ~~[WIP] Adds xblock-utils repository code into this repository~~ Adds xblock-utils repository code into this repository Sep 12, 2023

farhan requested a review from a team September 12, 2023 13:35

farhan force-pushed the farhan/merge-xblock-utils branch from 617d313 to 962adf4 Compare September 12, 2023 13:40

feanil reviewed Sep 12, 2023

View reviewed changes

farhan force-pushed the farhan/merge-xblock-utils branch 4 times, most recently from 8b1be5b to 4c47904 Compare September 13, 2023 13:22

farhan mentioned this pull request Sep 14, 2023

Merge openedx/xblock-utils library into openedx/XBlock #675

Closed

farhan force-pushed the farhan/merge-xblock-utils branch 2 times, most recently from 6c61401 to 279f341 Compare September 14, 2023 07:17

feanil reviewed Sep 14, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

setup.py Show resolved Hide resolved

xblock/test/test_plugin.py Show resolved Hide resolved

farhan requested a review from feanil September 14, 2023 13:54

feanil approved these changes Sep 14, 2023

View reviewed changes

bradenmacdonald reviewed Sep 14, 2023

View reviewed changes

xblock/utils/resources.py Show resolved Hide resolved

feanil reviewed Sep 19, 2023

View reviewed changes

xblock/utils/resources.py Outdated Show resolved Hide resolved

feanil reviewed Sep 19, 2023

View reviewed changes

xblock/__init__.py Outdated Show resolved Hide resolved

feanil requested a review from ormsbee September 19, 2023 13:14

ormsbee reviewed Sep 19, 2023

View reviewed changes

requirements/base.in Show resolved Hide resolved

ormsbee approved these changes Sep 19, 2023

View reviewed changes

farhan force-pushed the farhan/merge-xblock-utils branch 2 times, most recently from eecd419 to fdc1e17 Compare September 20, 2023 08:59

farhan mentioned this pull request Sep 21, 2023

Move xblock-utils repository docs #672

Merged

feanil reviewed Sep 22, 2023

View reviewed changes

farhan force-pushed the farhan/merge-xblock-utils branch 2 times, most recently from 2f1a9fb to f502eae Compare September 25, 2023 08:03

salman2013 self-requested a review September 25, 2023 10:45

salman2013 approved these changes Sep 25, 2023

View reviewed changes

feanil approved these changes Sep 25, 2023

View reviewed changes

CHANGELOG.rst Outdated Show resolved Hide resolved

farhan force-pushed the farhan/merge-xblock-utils branch 2 times, most recently from d0f0da7 to 21cd9a4 Compare September 25, 2023 15:19

farhan added 3 commits September 25, 2023 20:24

feat: Adds xblock-utils repository code into this repository

ed045db

docs: Move xblock utils repo docs into this repo

54f88d3

chore: Updating Python Requirements

d6b7b2d

farhan force-pushed the farhan/merge-xblock-utils branch from 21cd9a4 to d6b7b2d Compare September 25, 2023 15:25

feanil approved these changes Sep 25, 2023

View reviewed changes

farhan merged commit 2e506e4 into master Sep 26, 2023
4 of 6 checks passed

farhan deleted the farhan/merge-xblock-utils branch September 26, 2023 03:50

waza-ari mentioned this pull request Jan 25, 2024

xblock-utils package is deprecated citynetwork/markdown-xblock#38

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds xblock-utils repository code into this repository #669

Adds xblock-utils repository code into this repository #669

farhan commented Sep 7, 2023 •

edited

Loading

feanil left a comment

farhan commented Sep 11, 2023

feanil commented Sep 11, 2023

feanil left a comment

feanil left a comment

feanil left a comment

ormsbee left a comment

farhan commented Sep 21, 2023 •

edited

Loading

feanil left a comment

farhan commented Sep 25, 2023 •

edited

Loading

feanil left a comment

feanil left a comment

Adds xblock-utils repository code into this repository #669

Adds xblock-utils repository code into this repository #669

Conversation

farhan commented Sep 7, 2023 • edited Loading

feanil left a comment

Choose a reason for hiding this comment

farhan commented Sep 11, 2023

feanil commented Sep 11, 2023

feanil left a comment

Choose a reason for hiding this comment

feanil left a comment

Choose a reason for hiding this comment

feanil left a comment

Choose a reason for hiding this comment

ormsbee left a comment

Choose a reason for hiding this comment

farhan commented Sep 21, 2023 • edited Loading

feanil left a comment

Choose a reason for hiding this comment

farhan commented Sep 25, 2023 • edited Loading

feanil left a comment

Choose a reason for hiding this comment

feanil left a comment

Choose a reason for hiding this comment

farhan commented Sep 7, 2023 •

edited

Loading

farhan commented Sep 21, 2023 •

edited

Loading

farhan commented Sep 25, 2023 •

edited

Loading