Skip to content

refactor: move common/lib/capa/capa to xmodule/capa#30403

Merged
iamsobanjaved merged 1 commit intomasterfrom
iamsobanjaved/BOM-2582-move-common-lib-capa
Jul 21, 2022
Merged

refactor: move common/lib/capa/capa to xmodule/capa#30403
iamsobanjaved merged 1 commit intomasterfrom
iamsobanjaved/BOM-2582-move-common-lib-capa

Conversation

@iamsobanjaved
Copy link
Contributor

@iamsobanjaved iamsobanjaved commented May 16, 2022 •
edited
Loading

As part of the migration of common/lib packages, we are migrating this within xmodule directory.

Related PRs: jazkarta/edx-jsme#8

@iamsobanjaved iamsobanjaved force-pushed the iamsobanjaved/BOM-2582-move-common-lib-capa branch 2 times, most recently from 474a866 to 8708398 Compare May 17, 2022 08:22
@iamsobanjaved iamsobanjaved force-pushed the iamsobanjaved/BOM-2582-move-common-lib-capa branch 4 times, most recently from 1042e67 to 920676c Compare June 28, 2022 05:52
awais786
awais786 reviewed Jun 29, 2022
View reviewed changes
requirements/edx/base.txt Outdated
-e git+https://github.com/edx/django-wiki.git@1.1.0#egg=django-wiki
# via -r requirements/edx/github.in
-e git+https://github.com/jazkarta/edx-jsme.git@690dbf75441fa91c7c4899df0b83d77f7deb5458#egg=edx-jsme
-e git+https://github.com/iamsobanjaved/edx-jsme.git@c657f4d29f9082fbfd63490f415af00ec5556ecd#egg=edx-jsme
Copy link
Contributor

@awais786 awais786 Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats a reason for this change ?

Copy link
Contributor

@awais786 awais786 Jun 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iamsobanjaved/edx-jsme@c657f4d
thats the new commit hash with new changes.

@iamsobanjaved iamsobanjaved mentioned this pull request Jun 29, 2022
Closed
kdmccormick
kdmccormick reviewed Jun 29, 2022
View reviewed changes
Copy link
Member

@kdmccormick kdmccormick left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good so far. I am on vacation starting tomorrow until July 6 so don't wait for my approval on this one.

I hope you're able to get a quick review from Jazkarta on the edx-jsme PR. If you end up being blocked by that for a while, one workaround is to add back in temporary support for certain instances of import capa using compatibility modules like this one. You would need to make a new top-level capa directory, containing compatibility modules to match the ones that edx-jsme needs: capa/inputtypes.py, capa/responsetypes.py, and capa/correctmap.py. Hopefully, though, that won't be necessary....

@kdmccormick
Copy link
Member

kdmccormick commented Jul 7, 2022

Good news @iamsobanjaved : the edx-jsme dependency was just removed from edx-platform entirely: #30321. So that is not a blocker any more.

@iamsobanjaved iamsobanjaved force-pushed the iamsobanjaved/BOM-2582-move-common-lib-capa branch from 920676c to 1cd7453 Compare July 13, 2022 09:04
@iamsobanjaved iamsobanjaved marked this pull request as ready for review July 13, 2022 09:31
kdmccormick
kdmccormick requested changes Jul 13, 2022
View reviewed changes
Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I tested this in Tutor and it seems to work great.

A couple requests:

  • The docs build is currently failing because there are still references to common/lib/capa. Please search for and fix any remaining references to that folder, making sure that the docs build succeeds.
    • You can build the docs by entering an LMS shell, running pip install -r requirements/edx/doc.txt, and then running make docs.
  • Before merging please add details in your commit message, as well as a link to the public (openedx.atlassian.net) version of the ticket.

I've fixed the CI build by removing common-3 from the list of required checks.

@kdmccormick kdmccormick self-assigned this Jul 13, 2022
@iamsobanjaved
Copy link
Contributor Author

iamsobanjaved commented Jul 18, 2022

@kdmccormick i am facing same logs for my branch as of master branch ending with this

generating indices... genindex py-modindex done
writing additional pages... search done
copying images... [100%] testing/test_pyramid.png
copying static files... done
copying extra files... done
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded, 1565 warnings.

The HTML pages are in _build/html.
make[1]: Leaving directory '/edx/app/edxapp/edx-platform/docs/guides'
make -C docs/technical html
make[1]: Entering directory '/edx/app/edxapp/edx-platform/docs/technical'
sphinx-build -M html "." "_build"
Running Sphinx v5.0.2
WARNING: html_static_path entry '_static' does not exist
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 0 source files that are out of date
updating environment: [config changed ('featuretoggles_repo_version')] 3 added, 0 changed, 0 removed
reading sources... [100%] settings
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] settings
generating indices... genindex done
writing additional pages... search done
copying static files... done
copying extra files... done
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded, 1 warning.

The HTML pages are in _build/html.
make[1]: Leaving directory '/edx/app/edxapp/edx-platform/docs/technical'

Complete traceback: https://gist.github.com/iamsobanjaved/67874e03a90df2c317e00464123d94c4

@kdmccormick
Copy link
Member

kdmccormick commented Jul 18, 2022

@iamsobanjaved I think you either have old requirements installed or old Python code cached, from before the Discussion block was moved. Try restarting devstack and/or pulling new images. If that doesn't work, you could try running:

make clean                                # clears out cached .pyc files
make requirements                         # removes old requirements and installs new ones
pip install -r requirements/edx/docs.txt  # installs extra requirements for building docs

and then build the docs again.

@iamsobanjaved
refactor!: move common/lib/capa/capa to xmodule/capa
9eba9f9 
As part of dissolving our sub-projects in edx-platform, we are moving this package under the xmodule directory.
We have fixed all the occurences of import of this package and also fixed all documents related references.
This might break your platform if you have any reference of `import capa` or `from capa import` in your codebase or in any Xblock.

Ref: https://openedx.atlassian.net/browse/BOM-2582
@iamsobanjaved iamsobanjaved force-pushed the iamsobanjaved/BOM-2582-move-common-lib-capa branch from 1cd7453 to 9eba9f9 Compare July 19, 2022 07:28
@iamsobanjaved
Copy link
Contributor Author

iamsobanjaved commented Jul 19, 2022

@kdmccormick thanks, i was able to reproduce the failure locally and fixed that.

kdmccormick
kdmccormick approved these changes Jul 19, 2022
View reviewed changes
Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 🚀

@iamsobanjaved iamsobanjaved merged commit 06064b2 into master Jul 21, 2022
@iamsobanjaved iamsobanjaved deleted the iamsobanjaved/BOM-2582-move-common-lib-capa branch July 21, 2022 11:27
@iamsobanjaved iamsobanjaved mentioned this pull request Jul 21, 2022
Merged
@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jul 21, 2022

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Jul 21, 2022

EdX Release Notice: This PR has been deployed to the production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@awais786 awais786 awais786 approved these changes

@kdmccormick kdmccormick kdmccormick approved these changes

Assignees

@kdmccormick kdmccormick

@iamsobanjaved iamsobanjaved

Labels

None yet

Projects

Axim Engineering Tasks
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@iamsobanjaved @kdmccormick @edx-pipeline-bot @awais786