Modernize Python repo: pyproject.toml + uv + semantic-release

[salman2013]

Modernizes the Python tooling for DoneXBlock in three phases, aligning with the Open edX org standard established in openedx/sample-plugin.

Ticket: openedx/public-engineering#511
Parent ticket: openedx/public-engineering#506

Changes generated by Claude Sonnet 4.6 using the modernize-python-tooling skill, reviewed by human.

[openedx-webhooks]

Thanks for the pull request, @salman2013!

This repository is currently maintained by @openedx/axim-engineering.

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 approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To 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:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where 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:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

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:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.35%. Comparing base (5f248cd) to head (15282c2).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #388   +/-   ##
=======================================
  Coverage   55.35%   55.35%           
=======================================
  Files           3        2    -1     
  Lines          56       56           
  Branches        0        2    +2     
=======================================
  Hits           31       31           
  Misses         25       25           

Flag	Coverage Δ
unittests	55.35% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[irfanuddinahmad]

👋 Reviewed this against the same checklist we've been applying across the modernization effort. Nothing blocking — CI is fully green — just two small things worth a look:

1. pypa/gh-action-pypi-publish@release/v1 is a floating branch ref, not a pinned SHA. Same issue flagged on the other PRs in this batch — this is what caused openedx/xblocks-core's release to fail with docker: manifest unknown (https://github.com/openedx/xblocks-core/actions/runs/29327012856/job/87066591905). Worth pinning to a commit SHA. (Filed the same fix upstream: openedx/sample-plugin#54.)

2. fallback_version = "0.0.0.dev0" in [tool.setuptools_scm] — checked whether this could hit the same crash we found on openedx-events (that repo's fallback_version had the identical value, and a runtime .split(".")/int() parse of __version__ crashed 17 tests when it was hit). Confirmed done/__init__.py's __version__ isn't consumed anywhere else in this codebase, so it's safe here — no action needed, just flagging why the value looked familiar.

Non-blocking: major_on_zero/allow_zero_version aren't set in [tool.semantic_release] here — since this repo is already at 3.0.0 it'd be a no-op either way, but worth adding for consistency with the rest of the batch (protects against ever auto-jumping to 1.0.0 if the version is reset).

[irfanuddinahmad]

First review on this PR (no prior review activity). Verified everything against the actual current file content and live CI (all green) rather than just the diff. A few of these are the same recurring pattern already confirmed causing real breakage on sibling PRs in this same migration effort (fallback_version/fetch-depth, default-groups).

Things that are already done correctly and worth not re-litigating: all 6 action SHA pins are genuine (verified via the commits API, including one that needed resolving an annotated-tag object to its real commit), pypa/gh-action-pypi-publish correctly left at the stable tag, OIDC-only publish scoping, no License :: classifier conflicting with the SPDX field, src/ layout correctly adopted, CHANGELOG.rst's insertion marker present with no history dropped, and the Django version-matrix uses the correct [tool.uv].conflicts pattern rather than the shared-group anti-pattern found on 3 sibling repos.

[irfanuddinahmad]

tag_format isn't set here, so it defaults to python-semantic-release's "v{version}". This repo's actual releases are bare X.Y.Z (3.0.0, 2.5.0, ... -- confirmed via the tags API, and 3.0.0 matches PyPI's actual latest published version). Left at the default, PSR won't recognize any prior release as such.

There's also a genuinely confusing existing tag worth flagging separately: v3.0.0 (with the prefix) exists too, but it points to a different, later commit (21c502fb, a routine "chore: Upgrade Python requirements" commit from 11 days after the real 3.0.0 release at c116322a) -- not something this PR should try to silently fix, but worth a maintainer's attention since it could confuse PSR's history scan regardless of tag_format.

Suggested change

	major_on_zero = false
	allow_zero_version = true
	major_on_zero = false
	tag_format = "{version}"

[salman2013]

fixed

[irfanuddinahmad]

"0.0.0.dev0" is the exact fallback value that crashed 17 tests in a sibling repo (openedx-events) via tuple(map(int, __version__.split("."))) choking on the non-numeric dev0 segment. I checked every __version__ consumer in this repo (src/done/__init__.py, src/done/done.py, docs/source/conf.py) -- none int-parse it, so it's not an active crash risk here today, but there's no reason to keep a value from the banned-pattern class when a plain int-parseable one works just as well.

Suggested change

	fallback_version = "0.0.0.dev0"
	fallback_version = "0.0.0"

[salman2013]

fixed

[irfanuddinahmad]

Missing fetch-depth: 0. Without it, this is a shallow, tag-less clone, so setuptools-scm can't see any tags during test runs and silently falls back to fallback_version (see the pyproject.toml comment) on every ordinary CI run. release.yml's own checkout correctly has this set; this job's doesn't.

Suggested change

	- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
	- uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
	with:
	fetch-depth: 0

[salman2013]

fixed

[irfanuddinahmad]

This should be wrapped in try/except PackageNotFoundError -- as written, any environment where done-xblock's dist-info isn't discoverable at import time (some editable-install/plugin-loading edge cases) raises unhandled and takes down the whole XBlock module, which is a harder failure than the hardcoded string this replaced.

Suggested change

	from importlib.metadata import PackageNotFoundError, version
	from .done import DoneXBlock
	try:
	__version__ = version("done-xblock")
	except PackageNotFoundError:
	__version__ = "unknown"

[salman2013]

I removed this version as UV manages by own.

[irfanuddinahmad]

ruff is declared as a dependency in the quality group and fully configured ([tool.ruff]/[tool.ruff.lint]/[tool.ruff.format] in pyproject.toml), but this target never actually invokes it -- only pylint runs. Was ruff meant to run here too (matching the #511 XBlocks track's adoption of it elsewhere, e.g. xblocks-core), or was pylint intended to stay as the sole linter with ruff left over from an earlier draft? Either way it's currently dead config -- worth wiring in or dropping.

[salman2013]

We are not adding ruff in this scope because it needs to add more files formatting, so i removed that.

[irfanuddinahmad]

A dev-named dependency-group exists elsewhere in this file, and every invocation in this repo (uv sync --group ci, uv sync --group dev, tox's dependency_groups =) already names an explicit group -- exactly the condition where default-groups = [] is needed. Without it, uv sync --group ci also implicitly syncs the full dev superset alongside whatever group was actually requested.

Suggested change

	[tool.uv]
	[tool.uv]
	package = true
	# Every `uv sync`/`uv run` invocation in this repo names an explicit --group.
	# Without this, uv's implicit default group (named "dev") would be synced *in
	# addition* to whatever --group is passed, defeating the point of having
	# separate groups.
	default-groups = []
	conflicts = [
	[{group = "test"}, {group = "django42"}],
	]

[salman2013]

fixed

[irfanuddinahmad]

Minor: "AGPL-3.0" is a deprecated SPDX identifier (superseded by AGPL-3.0-only/AGPL-3.0-or-later). Not a build failure (no conflicting License :: classifier is present), just imprecise -- worth checking the LICENSE file's actual text to pick the right variant.

[salman2013]

fixed

[irfanuddinahmad]

A couple of minor, non-blocking notes.

[irfanuddinahmad]

These (and test/covreport below) call pylint/python/twine bare, no uv run — only bites if someone runs them directly instead of through tox. Same assumption existed pre-migration too, so not a new issue, just worth a follow-up sometime.

[salman2013]

fixed

[irfanuddinahmad]

Heads up, RTD now has native uv support (method: uv, command: sync) if you ever want doc deps resolved from uv.lock too — not necessary for just Sphinx + the theme though, this is fine as is.

[salman2013]

fixed

[feanil]

This should be replaced with a get_version call and the variable should still be set for convenience/compatibility.

[feanil]

I thought we were not going to add changelogs since they can't be updated by python-semantic-release the way we have it setup.

[salman2013]

You're right — python-semantic-release can't auto-update it in our current setup . Removed the file and the
[tool.semantic_release.changelog] config block from pyproject.toml as well, which auto-generated this file.

[feanil]

Is this file new or is this config being moved from somewhere else, I don't see the file this is coming from if it's moving.

[salman2013]

Yes this is a new addition, just added in parity of other repo like xblock-core to show the code coverage in the checks list. should i remove this? as it is not a requirement of modernization.

[feanil]

Yes, codecov has default configuration which is fine in most cases. We only need to override it sometimes.

[salman2013]

I have removed it.

[salman2013]

@feanil I believe its ready for another pass.

[farhan]

few changes, mostly looks good

[farhan]

allow_zero_version and major_on_zero form a zero-version guard that only applies to 0.x repos — this repo is at 3.0.0.

[farhan]

Add # pragma: no cover — the except branch is unreachable in tests (the package is always installed) and codecov will flag it as uncovered:

except PackageNotFoundError:  # pragma: no cover

[farhan]

we should update it to latest version

[farhan]

We should add docs environment

I tested it locally, make docs is failing

[farhan]

this is pending, make sure we run docs in the ci as well.

[salman2013]

This is already done in this commit

[salman2013]

[farhan]

Ah, docs env is defined at the bottom of the file

Please add it in this envlist so a bare tox run covers it too and its easy to read the file

[farhan]

We can remove this LICENSE declaration now

[farhan]

We have decided to not to drop the make file targets, we can provide their alternatives.

[farhan]

We should docs as well in it.

[irfanuddinahmad]

Hey @salman2013 — nudge on farhan's review from 8/25 (still CHANGES_REQUESTED, no changes pushed since). The # pragma: no cover on except PackageNotFoundError in particular is why codecov/project is currently red. The other four (zero-version guard, release.yml version bump, docs tox env, MANIFEST.in LICENSE line) are still open too.

[farhan]

📦 Packaging check — asset diff vs master

I built the wheel + sdist from this branch and from master and compared the contents. Core static assets (static/, templates/, public/ — png/css/js/html) are identical between the two ✅, but the installed wheel has a couple of differences worth a look.

✅ Added in the PR wheel (not in master's wheel)

• done/tests/__init__.py, done/tests/test_nothing.py — the test package is now shipped into site-packages. [tool.setuptools.packages.find] under src/ picks up done.tests, and exclude-package-data only strips data files, not .py modules. Master's wheel didn't install these.
• done/conf/locale/config.yaml — benign (legitimate package data).
• done_xblock-3.0.0.dist-info/licenses/LICENSE — LICENSE just relocated to the PEP 639 licenses/ subdir (not new content).

❌ Dropped in the PR wheel (was in master's wheel)

• done_xblock-3.0.0.dist-info/NOTICE — NOTICE is no longer shipped anywhere in the installed wheel. license-files = ["LICENSE"] omits it. (It still lands in the sdist via include NOTICE, but not in the wheel that users actually install.)
• done_xblock-3.0.0.dist-info/LICENSE — only moved to licenses/LICENSE (above), so not truly lost.

Lower-impact: the sdist is now bloated

The sdist now ships nearly every repo file — .github/, docs/, Dockerfile, tox.ini, uv.lock, scripts/, catalog-info.yaml, pylintrc, root screenshots, etc. This is because setuptools-scm includes all git-tracked files by default and the new MANIFEST.in (just include NOTICE) has no prune/exclude rules. It doesn't affect installs, but it's untidy and would let htmlcov/ / coverage.xml / .coverage leak into a release build made from a post-test tree.

Suggested fix (one small packaging commit)

1. Keep NOTICE in the wheel: license-files = ["LICENSE", "NOTICE"]
2. Exclude tests from the wheel: [tool.setuptools.packages.find] → exclude = ["*.tests*", "tests*"]
3. Add prune rules to MANIFEST.in (.github, docs, htmlcov, coverage.xml, .coverage) to trim the sdist and prevent artifact leakage.

Repro

python -m build   # this branch
git worktree add /tmp/m master && (cd /tmp/m && python -m build --no-isolation -o /tmp/mb)
diff <(unzip -l /tmp/mb/*.whl | awk 'NR>3{print $4}' | sort) \
     <(unzip -l dist/*.whl    | awk 'NR>3{print $4}' | sort)

[farhan]

2 important points to address

rest all seems good

[farhan]

Set vcs_release: "false" to match the sample-plugin standard

Suggested change

	changelog: "false"
	vcs_release: "false"

[farhan]

Paired with vcs_release: "false" above: once PSR no longer publishes the release, nothing creates it. Add an immutable-safe step here, before the artifact upload, that creates the release as a draft, attaches the dists, then publishes — the only ordering immutable releases allow. This mirrors sample-plugin:

      - name: Create GitHub Release with Assets
        if: steps.release.outputs.released == 'true'
        env:
          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
          RELEASE_NOTES: ${{ steps.release.outputs.release_notes }}
          TAG: ${{ steps.release.outputs.tag }}
        run: |
          printf '%s' "$RELEASE_NOTES" > "$RUNNER_TEMP/release_notes.md"
          gh release create "$TAG" \
            --verify-tag --title "$TAG" \
            --notes-file "$RUNNER_TEMP/release_notes.md" \
            dist/*

[farhan]

this is pending, make sure we run docs in the ci as well.

[farhan]

2 nits and a change request

[farhan]

Ah, docs env is defined at the bottom of the file

Please add it in this envlist so a bare tox run covers it too and its easy to read the file

[farhan]

Bump artifact actions to the latest: upload-artifact → v7.0.1 (043fb46d…), download-artifact → v8.0.1 (3e5f45b2…).

[salman2013]

fixed