Conversation
|
Thanks for the pull request, @aznszn! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Submit a signed contributor agreement (CLA)
If you've signed an agreement in the past, you may need to re-sign. Once you've signed the CLA, please allow 1 business day for it to be processed. 🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
2ad9ce4 to
bd84eb1
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes Django 4.2 as a supported/tested framework version for the project, updating CI, tox environments, and package metadata accordingly.
Changes:
- Removed the Django 4.2 tox factor/env and CI job entry, leaving Django 5.2 as the tested Django version.
- Updated package classifiers to no longer advertise Django 4.2 support.
- Added a changelog entry noting the support drop.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tox.ini |
Removes django42 factor and narrows the envlist to Django 5.2. |
.github/workflows/ci.yml |
Drops django42 from the GitHub Actions matrix. |
setup.py |
Removes the Framework :: Django :: 4.2 classifier. |
CHANGELOG.rst |
Documents the removal of Django 4.2 support under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [tox] | ||
| envlist = py{312}-django{42, 52}, docs, quality | ||
| envlist = py{312}-django{52}, docs, quality |
There was a problem hiding this comment.
The tox envlist defines a single Django test environment named py312-django52, but the CI workflow matrix uses TOXENV=django52. django52 is not a defined tox environment in this repo (no [testenv:django52]), so tox will error in CI. Align the env naming by either updating envlist/adding a django52 env, or updating CI to run py312-django52.
| matrix: | ||
| os: [ubuntu-latest] | ||
| python-version: ['3.12'] | ||
| toxenv: [quality, docs, django42, django52] | ||
| toxenv: [quality, docs, django52] | ||
|
|
There was a problem hiding this comment.
The workflow matrix uses toxenv: ... django52, but tox.ini doesn't define a django52 environment (it defines py312-django52). With TOXENV=django52, tox will fail due to an unknown environment. Align the matrix value with the tox env name (and update the later coverage if that checks matrix.toxenv=='django52' accordingly), or add a matching django52 tox env.
| classifiers=[ | ||
| 'Development Status :: 3 - Alpha', | ||
| 'Framework :: Django', | ||
| 'Framework :: Django :: 4.2', | ||
| 'Framework :: Django :: 5.2', | ||
| 'Intended Audience :: Developers', |
There was a problem hiding this comment.
This PR drops Django 4.2 support and the package classifiers now only advertise Django 5.2, but install_requires is still loaded from requirements/base.in, which currently allows Django>=2.2. That means pip can install this project alongside unsupported Django versions despite the advertised support. Consider updating the Django requirement (in requirements/base.in and/or via constraints) to enforce the minimum supported Django version for releases that drop 4.2.
- Pin Django requirement to >=5.2,<5.3 in base.in - Update CI toxenv from 'django52' to 'py312-django52' to match tox config
Description: Dropped Django 4.2 support. Removed Django 4.2 from CI matrix, tox environments, and setup.py classifiers.
Issue: openedx/public-engineering#458
JIRA: N/A
Dependencies: N/A
Merge deadline: N/A
Installation instructions: N/A
Reviewers:
Merge checklist:
Post merge: