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

Paver Removal 3/3 #34832

Merged
merged 4 commits into from
Jan 2, 2025
Merged

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented May 21, 2024

Description

This fully removes Paver from edx-platform and updates some documentation accordingly. See individual commits for more detail.

  • BREAKING CHANGE: Removes all remaining Paver commands including
    pavelib/prereqs.py:* and pavelib/assets.py:*.

  • BREAKING CHANGE: Removes ./manage.py [lms|cms] compile_sass, which
    was just a wrapper around Paver commands.

  • BREAKING CHANGE: Removes paver.txt. Operators should install testing.txt
    instead.

  • BREAKING CHANGE: Removes libsass from requirements/edx/base.txt.
    Operators will need to install requirements/edx/assets.txt in order to
    compile Sass.

Note that the build!: rejigger npm run test commands commit is technically a breaking change, but those commands were first added a week ago and were not documented, so I don't feel like we need to raise this as BREAKING CHANGE.

Supporting information

This plan has been accepted and communicated via the DEPR process:

Previous PRs (blocking this one):

Testing instructions

I built the Tutor openedx and openedx-dev images with this branch. I smoke tested LMS and CMS in both modes, and ensure that I could toggle between Indigo and the default theme.

I inspected the JS Test logs to ensure that we're still running 6 test suites (jest, cms-vanilla, cms-require, xmodule-vanilla, common-vanilla, common-require).

Finally, you can check out the updated edx-platform testing guide here: https://github.com/kdmccormick/edx-platform/blob/kdmccormick/paver-rm-3/docs/concepts/testing/testing.rst. Using Tutor, I installed the tutor-contrib-legacy-js plugin (from a branch: https://github.com/kdmccormick/openedx-tutor-plugins/tree/kdmccormick/test-legacy-js/plugins/tutor-contrib-test-legacy-js) and ran JS tests locally.

@kdmccormick
Copy link
Member Author

@feanil If you have some spare cycles to review this, would you like do the honors?

@kdmccormick
Copy link
Member Author

kdmccormick commented Dec 18, 2024

I'm hoping to merge a subset of this PR ASAP:

The rest of this PR (all the breaking parts) will have to wait for a bit while we sort out a regression in edx-proctoring.

@kdmccormick kdmccormick marked this pull request as draft December 18, 2024 15:52
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.

These changes look good, can we also update xmodule/tests/__init__.py and lms/envs/minimal.yml to not mention paver. The rest of the mentions I think will get cleaned up in future cleanup that we have planned around devstack settings or it is in historic documents.

Good to merge once those last two files are updated from my perspective.

@kdmccormick kdmccormick force-pushed the kdmccormick/paver-rm-3 branch from 2aa06f4 to d9d3f1e Compare January 2, 2025 18:26
@kdmccormick kdmccormick marked this pull request as ready for review January 2, 2025 18:26
@kdmccormick
Copy link
Member Author

@feanil Done, and thank you!

kdmccormick and others added 4 commits January 2, 2025 13:34
BREAKING CHANGE: Removes all remaining Paver commands including
`pavelib/prereqs.py:*` and `pavelib/assets.py:*`.

BREAKING CHANGE: Removes `./manage.py [lms|cms] compile_sass`, which
was just a wrapper around Paver commands.

BREAKING CHANGE: Removes paver.txt. Operators should install testing.txt
instead.

Part of: openedx#34467
There are three packages which edx-platform needs in order to run which
were being installed transitively via paver.in. Since we are removing
paver.in, these dependencies need to be transferred into kernel.in:

* psutil
* pymemcache
* wrapt

Part of: openedx#34467
BREAKING CHANGE: Removes libsass from requirements/edx/base.txt.
Operators will need to install requirements/edx/assets.txt in order to
compile Sass.

Commit generated by workflow `kdmccormick/edx-platform/.github/workflows/compile-python-requirements.yml@refs/heads/master`

Part of: openedx#34467
@kdmccormick kdmccormick force-pushed the kdmccormick/paver-rm-3 branch from d9d3f1e to b509e55 Compare January 2, 2025 18:36
@kdmccormick kdmccormick enabled auto-merge (rebase) January 2, 2025 18:39
@kdmccormick kdmccormick merged commit 88b8da3 into openedx:master Jan 2, 2025
48 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/paver-rm-3 branch January 2, 2025 19:05
@edx-pipeline-bot
Copy link
Contributor

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

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.

3 participants