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 2/3 #34830

Closed
Closed

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented May 21, 2024

Description

  • BREAKING CHANGE: Removes all remaining Paver commands except the miscellaneous quality checks. Removed commands:
    • pavelib/prereqs.py:*
    • pavelib/assets.py:*
  • BREAKING CHANGE: Removes ./manage.py [lms|cms] compile_sass, which was just a wrapper around Paver commands.
  • Removes paver from base requirements, as it's only needed for testing now.

Supporting info

This is part of:

Previous work (blocking this):

Next PRs:

Testing instructions

None. For the community deployment, this is all dead code. The replacement (npm run) was thoroughly tested as part of the linked DEPR ticket.

Merge considerations:

  • We're eager to merge this as it will deliver a ~1-2min build-time improvement for prod images, as libsass will no longer be part of the platform's base requirements. No change for dev images, though, as libsass is still needed by development requirements.

@kdmccormick kdmccormick force-pushed the kdmccormick/paver-rm-2 branch 2 times, most recently from 3f77cf6 to b14b788 Compare May 22, 2024 14:26
@kdmccormick kdmccormick force-pushed the kdmccormick/paver-rm-2 branch 2 times, most recently from 4d524cf to c184394 Compare May 22, 2024 16:00
kdmccormick added a commit that referenced this pull request May 22, 2024
As of Python 3.3, the 3rd-party `mock` package has been subsumed into the
standard `unittest.mock` package. Refactoring tests to use the latter will
allow us to drop `mock` as a dependency, which is currently coming in
transitively through requirements/edx/paver.in.

We don't actually drop the `mock` dependency in this PR. That will happen
naturally in:

* #34830
@kdmccormick kdmccormick force-pushed the kdmccormick/paver-rm-2 branch from c184394 to 5f8ca19 Compare November 12, 2024 19:28
These packages were installed transitively through paver.in, but they
are used as direct dependencies in edx-platform application code:

* psutil
* pymemcache
* wrapt

Since we are demoting paver.in to be a dev-only dependency (with plans
to remove paver.in entirely), we need to make those three packages
explicit dependencies in kernel.in

Part of: openedx#34467
@kdmccormick kdmccormick force-pushed the kdmccormick/paver-rm-2 branch from ba91de2 to e0df3c4 Compare November 12, 2024 21:10
Commit generated by workflow `kdmccormick/edx-platform/.github/workflows/compile-python-requirements.yml@refs/heads/master`

Co-authored-by: Kyle McCormick <[email protected]>
@kdmccormick kdmccormick marked this pull request as ready for review November 12, 2024 21:30
@kdmccormick kdmccormick requested a review from feanil November 13, 2024 12:21
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.

Looks good but don't forget to add details to the second commit message before merging.

@kdmccormick
Copy link
Member Author

Will do @feanil .

Per discussion on the linked DEPR, we will be delaying merge until at least Monday morning.

@kdmccormick
Copy link
Member Author

This intermediate step is unneccessary/outdated, since we have removed the Paver commands in: #35159

Closing in favor of the final step, #34832

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.

2 participants