Skip to content

refactor: move package source to src/ layout#32

Merged
kdmccormick merged 2 commits intomainfrom
feanil/src-layout
May 4, 2026
Merged

refactor: move package source to src/ layout#32
kdmccormick merged 2 commits intomainfrom
feanil/src-layout

Conversation

@feanil
Copy link
Copy Markdown
Contributor

@feanil feanil commented Apr 30, 2026

Summary

  • Moves backend/sample_plugin/ to backend/src/sample_plugin/ to adopt the standard src/ layout
  • Adds where = ["src"] to [tool.setuptools.packages.find] in backend/pyproject.toml
  • Updates backend/MANIFEST.in to use the new path and removes stale requirements/ lines
  • Updates backend/tox.ini quality commands (pylint, pycodestyle, pydocstyle, isort) to reference src/sample_plugin

Motivated by feedback on openedx/public-engineering#506: the src/ layout prevents accidental imports from the source tree during tests and fixes editable-install compatibility with mypy and other static analysis tools.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.40%. Comparing base (0884981) to head (e87b157).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #32      +/-   ##
==========================================
+ Coverage   88.23%   88.40%   +0.17%     
==========================================
  Files          13       14       +1     
  Lines         408      414       +6     
  Branches       16       16              
==========================================
+ Hits          360      366       +6     
  Misses         40       40              
  Partials        8        8              
Flag Coverage Δ
unittests 88.40% <ø> (+0.17%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

Moves sample_plugin/ into src/ to adopt the standard src-layout, which
prevents accidental imports from the source tree during tests and fixes
editable-install compatibility with mypy and other tools that rely on
proper package discovery.

- backend/sample_plugin/ moved to backend/src/sample_plugin/
- backend/pyproject.toml: add `where = ["src"]` to [tool.setuptools.packages.find]
- backend/MANIFEST.in: update recursive-include path, remove stale requirements/ lines
- backend/tox.ini: update quality tool invocations to use src/sample_plugin path

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@bradenmacdonald bradenmacdonald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @feanil ! I tested this by making an editable install into my edx-platform venv, and it seems to be working well. I left some suggestions but feel free to save them for a future PR.

Comment thread backend/pyproject.toml
where = ["src"]
include = ["sample_plugin*"]
exclude = ["sample_plugin.tests*"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think if you put the following in, you can delete MANIFEST.in:

Suggested change
[tool.setuptools.package-data]
# Include these data files when building the wheel, if any are present in src/
sample_plugin = ["**/*.html", "**/*.png", "**/*.gif", "**/*.js", "**/*.css", "**/*.jpg", "**/*.jpeg", "**/*.svg"]

Or, since we're using setuptools-scm, we could use:

[tool.setuptools]
include-package-data = true

to pull in all files in src tracked by git without needing to maintain this separate list of file extensions.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, whether using MANIFEST.in or package-data in pyproject.toml, I would recommend creating an empty py.typed file at src/sample_plugin/py.typed and then adding "py.typed", to this list, e.g. sample_plugin = ["py.typed", "**/*.html", "**/*.png", ... so that mypy will pick up any type definitions if other python projects are importing and using this code. Not so necessary for a plugin but very important for any reusable libraries.

Comment thread backend/pyproject.toml Outdated
[tool.setuptools.packages.find]
where = ["src"]
include = ["sample_plugin*"]
exclude = ["sample_plugin.tests*"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
exclude = ["sample_plugin.tests*"]

I think this exclude is not necessary when you have where = ["src"] and the tests are outside of the src folder.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove include = ["sample_plugin*"] too, since the existence of a src folder makes me expect that anything within src should be included.

Comment thread backend/tox.ini
isort --check-only --diff tests test_utils sample_plugin manage.py test_settings.py
pycodestyle src/sample_plugin tests manage.py
pydocstyle src/sample_plugin tests manage.py
isort --check-only --diff tests test_utils src/sample_plugin manage.py test_settings.py
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: replace with ruff ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will follow up #33

Copy link
Copy Markdown

@farhan farhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add following in project.toml and remove it and readme declaration from Manifest.in
license-files = ["LICENSE.txt"]

Copy link
Copy Markdown
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, nothing to add beyond what Farhan and Braden have said.

@kdmccormick kdmccormick enabled auto-merge (squash) May 4, 2026 18:13
@kdmccormick kdmccormick disabled auto-merge May 4, 2026 18:13
@kdmccormick kdmccormick merged commit c197961 into main May 4, 2026
7 checks passed
@kdmccormick kdmccormick deleted the feanil/src-layout branch May 4, 2026 18:13
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.

4 participants