-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Removed requirements txt files from project. #9842
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR migrates dependency management from multiple requirements/*.txt files to a centralized pyproject.toml configuration, organizing dependencies into optional dependency groups (testing, optional, documentation, packaging). This modernizes the project's dependency management approach using PEP 621 standards.
Key Changes
- Migrated all dependencies from 5 requirements files into pyproject.toml optional-dependencies groups
- Updated tox.ini to use pyproject.toml optional dependencies instead of requirements files
- Updated installation instructions in documentation and CI workflows
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Added four optional-dependencies groups (documentation, optional, packaging, testing) with migrated dependencies |
| tox.ini | Updated all test environments to install dependencies using .[group] syntax instead of -r requirements/*.txt |
| docs/community/contributing.md | Updated installation instructions to use pyproject.toml optional dependencies |
| .github/workflows/main.yml | Updated CI workflow to install dependencies from pyproject.toml |
| requirements.txt | Removed root requirements file |
| requirements/requirements-*.txt | Removed all requirements directory files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0699662 to
54a17b2
Compare
browniebroke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using optional-dependencies for that is IMO the wrong way to solve this.
c87972b to
2f2086d
Compare
browniebroke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nearly there
2f2086d to
e5a1a57
Compare
e5a1a57 to
70b7949
Compare
70b7949 to
c847be5
Compare
|
I think there is still a mention here: django-rest-framework/docs/community/project-management.md Lines 84 to 88 in c847be5
This one should be removed:
And this one should be changed to pyproject.toml:
Also this:
|
* Removed references to `requirements.txt` in GitHub Actions workflows. * Updated `mkdocs-deploy.yml` and `main.yml` to install dependencies using `pyproject.toml`. * Cleaned up documentation to remove mentions of the `requirements` folder.
Thanks for catching this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking this on, looks good to me
PS: will leave this open for a few days to give time to other maintainers to take a look
Project's URLs should be in the [project] table
|
Looking at the logs more closely, I think something isn't right: The |
may be bcs of this step |
|
Right so tox does:
So I initially assumed step 3 overrides the version from step 1, but it's actually overridden in step 2 (by running So one of our testing or optional dependency is requesting a Django version newer than 4.2, causing it to upgrade. It's a bit annoying that the incompatibility is not flagged more loudly 😞 |
peterthomassen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar enough with the various ways of managing requirements so that I don't feel competent to do a proper review. I'll be OK with what the other maintainers decide. Thank you!
browniebroke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've checked the various CI jobs and they run with the right Python and Django versions. Docs job works as expected as well
we have migrated to pyproject.toml so now I think it is safe to remove requirements folder and requirements.txt file bcs pyproject.toml could take care of those installations. I have shifted those all dependencies in pyproject.toml.