Repository navigation
feat: modernize Python tooling (pyproject.toml + uv + semantic-release) - #481
irfanuddinahmad wants to merge 7 commits into
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the pull request, @irfanuddinahmad! This repository is currently unmaintained. 🔘 Find a technical reviewerTo get help with finding a technical reviewer, reach out to the community contributions project manager for this PR:
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:
🔘 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. |
…igrations Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| [project] | ||
| name = "edx-rbac" | ||
| description = "Library to help managing role based access controls for django apps" | ||
| requires-python = ">=3.11" |
There was a problem hiding this comment.
This publishes Requires-Python: >=3.11, which 3.0.0 didn't declare. The classifiers and the CI matrix only cover 3.12, and the py311 tox envs were never run in CI, so 3.11 would be advertised without being tested. Can you make it >=3.12 and drop py311 from the envlist, like help-tokens#350 did?
There was a problem hiding this comment.
>=3.11 matches master's tox envlist, which still has py311 envs. Without it, uv locks only for whatever Python runs uv lock, and the py311 envs break. Dropping 3.11 support would be a separate change.
| include requirements/base.in | ||
| recursive-include edx_rbac *.html *.png *.gif *js *.css *jpg *jpeg *svg *py | ||
| include requirements/constraints.txt | ||
| recursive-include src/edx_rbac *.html *.png *.gif *js *.css *jpg *jpeg *svg *py |
There was a problem hiding this comment.
The sdist grew from 49 files to 81 here. setuptools-scm adds every git-tracked file, so it now ships uv.lock, .github/, docs/, tests/, Makefile, tox.ini, pytest.ini, lint configs and more. Can you add excludes here?
prune .github
prune docs
prune tests
prune test_utils
prune locale
prune htmlcov
exclude uv.lock Makefile tox.ini pytest.ini pylintrc pylintrc_tweaks codecov.yml openedx.yaml db_keyword_overrides.yml manage.py test_settings.py .gitignore .annotation_safe_list.yml .pii_annotations.yml
global-exclude .coverage coverage.xml
There was a problem hiding this comment.
I'd keep this out of the migration PR. None of the merged migrations added sdist excludes, so it fits better as a separate follow-up.
| include-package-data = false | ||
|
|
||
| [tool.setuptools.dynamic] | ||
| readme = {file = ["README.rst", "CHANGELOG.rst"], content-type = "text/x-rst"} |
There was a problem hiding this comment.
Since the changelog is turned off in release.yml, CHANGELOG.rst won't be updated anymore (it stops at 3.0.0), but it's still appended to the PyPI description here. Can you drop it from readme and add a short note at the top of CHANGELOG.rst pointing to GitHub Releases?
There was a problem hiding this comment.
Done, dropped it from readme and added a note at the top of CHANGELOG.rst pointing to GitHub Releases.
| env: | ||
| TOXENV: ${{ matrix.toxenv }} | ||
| run: tox | ||
| run: uv run --locked tox |
There was a problem hiding this comment.
I think we can avoid --locked with uv run, with uv sync its already there
| # the checks that were required to merge -- and picks up any package added to | ||
| # ci.yml later without needing a change here. | ||
| run_ci: | ||
| uses: ./.github/workflows/ci.yml |
There was a problem hiding this comment.
run_ci calls ci.yml without secrets: inherit, so on master pushes the Codecov step runs without CODECOV_TOKEN, and it has fail_ci_if_error: true. sample-plugin and XBlock are set up the same way, so tokenless upload is probably fine, but has this been confirmed? If the upload fails, release won't run.
There was a problem hiding this comment.
Yes, openedx-chem has the same setup (no secrets: inherit, fail_ci_if_error: true) and the Codecov upload passed in its v2.0.1 release run.
CHANGELOG.rst is no longer updated now that release notes go to GitHub Releases, so stop appending it to the long description and note that at the top of the file. uv sync --locked already guards the lockfile in CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Moves edx-rbac from setup.py/pip-tools to pyproject.toml, uv and python-semantic-release.
Part of openedx/public-engineering#512 (parent openedx/public-engineering#506).
pyproject.toml, and setuptools-scm reads the version from git tags.setup.py,setup.cfgand the hard-coded__version__string are gone;__version__anddocs/conf.pynow useimportlib.metadata.requirements/is replaced by dependency groups anduv.lock. The Django cap anddiff-cover==4.0.0sit in[tool.uv].constraint-dependencies, so the published dependencies are unchanged.tox-uvwith theuv-venv-lock-runner. CI installs withsetup-uvand runs the same matrix as before (quality, django42, django52, check_keywords).release.ymlcomes from openedx/sample-plugin and publishes to PyPI through trusted publishing. It replacespublish_pypi.yml, which used a token.src/edx_rbac/.Before merging
edx-rbacwith repositoryopenedx/edx-rbac, workflowrelease.yml.The latest tag matching PyPI is
v3.0.0(3.0.0 on PyPI). It points at the same commit as the bare3.0.0tag. PSR ignores the bare tag and treatsv3.0.0as the last release, so the first automated release will bev3.1.0.Testing
tox -e quality,django42,django52,check_keywords,docspasses locally on Python 3.12: 93 tests under each Django version, 84% coverage, the same as master.py3-none-anyinstead ofpy2.py3) and the metadata fields (License-ExpressionAGPL-3.0-or-later, author Open edX Project, Requires-Python>=3.11) differ. The sdist still ships README, CHANGELOG, LICENSE and AUTHORS, andtwine checkpasses.Reviewer notes
include-package-data = falseplus an explicittemplates/**package-data entry keeps the wheel identical to 3.0.0. Withtrue, setuptools-scm would also shipconf/locale/config.yaml.requires-python = ">=3.11"matches thepy311env that tox.ini still lists.edx-rbac/directory. They now point atsrc/edx_rbac.🤖 Generated with Claude Code