Skip to content
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

refactor: make xmodule & openedx independent of code in cms #27874

Merged
merged 3 commits into from
Jun 9, 2021

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Jun 7, 2021

Description

In BOM-2576 (see draft PR), we will begin constraining imports in edx-platform. Two such constraints will be:

  • Modules in ./common/lib/xmodule/xmodule may not import from ./cms.
  • Modules in ./openedx may not import from ./cms.

This PR makes three small refactorings (separated by commit) that make the above constraints true.

Why?

The xmodule and openedx packages both contain code that is common between LMS and CMS. The cms package is, of course, contains CMS-specific code.

So, one prerequisite to extracting CMS into an independent service will be making sure that lms, common, openedx, and the projects in common/lib (xmodule, capa, etc.) are not coupled to any CMS-specific features.

Testing instructions

None

Deadline

None

@kdmccormick
Copy link
Member Author

jenkins runs python

@kdmccormick
Copy link
Member Author

jenkins run python

Base automatically changed from kdmccormick/import-as-xmodule to master June 8, 2021 12:45
@kdmccormick kdmccormick force-pushed the kdmccormick/xmodule-no-cms branch from 44c54cd to 53b5cf8 Compare June 8, 2021 13:42
kdmccormick and others added 3 commits June 8, 2021 10:46
This is a minor refactoring in order to remove
a code dependency of ./opendedx on ./cms
This is a minor refactoring in order to remove
a code dependency of ./xmodule on ./cms
This is a minor refactoring in order to remove
a code dependency of ./xmodule on ./cms
@kdmccormick kdmccormick force-pushed the kdmccormick/xmodule-no-cms branch from 53b5cf8 to 7a26ec6 Compare June 8, 2021 14:46
@kdmccormick kdmccormick changed the title refactor: remove dependency of xmodule on cms refactor: remove dependency of xmodule/openedx on cms Jun 8, 2021
@kdmccormick kdmccormick changed the title refactor: remove dependency of xmodule/openedx on cms refactor: remove dependency of xmodule & openedx on cms Jun 8, 2021
@kdmccormick kdmccormick changed the title refactor: remove dependency of xmodule & openedx on cms refactor: make xmodule & openedx independent of code in cms Jun 8, 2021
@edx-status-bot
Copy link

Your PR has finished running tests. There were no failures.

@kdmccormick kdmccormick marked this pull request as ready for review June 8, 2021 15:28
@awaisdar001 awaisdar001 self-requested a review June 8, 2021 16:32
Copy link
Contributor

@awaisdar001 awaisdar001 left a comment

Choose a reason for hiding this comment

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

Looks good to me. 👍

@kdmccormick kdmccormick merged commit de88595 into master Jun 9, 2021
@kdmccormick kdmccormick deleted the kdmccormick/xmodule-no-cms branch June 9, 2021 13:15
@edx-pipeline-bot
Copy link
Contributor

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 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants