-
Notifications
You must be signed in to change notification settings - Fork 14
refactor: PEP-518 and use uv
#42
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
base: main
Are you sure you want to change the base?
Conversation
Everything is working well locally, will need to see what happens in GitHub Actions and likely will need to make some tune-ups.
WalkthroughThe project migrated to a PEP 518-compliant build and development workflow centered around Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Shell/CI
participant uv
participant Python/Tools
Developer->>Shell/CI: Run make/lint/test/docs commands
Shell/CI->>uv: Execute via uv run/sync/build/publish
uv->>Python/Tools: Launch Python, tox, sphinx, etc.
Python/Tools-->>uv: Perform requested actions
uv-->>Shell/CI: Return results
Shell/CI-->>Developer: Output/logs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 3
🧹 Nitpick comments (10)
.flake8 (1)
1-3
: Centralized Flake8 config—good start
Consolidating lint rules in a dedicated.flake8
file aligns with PEP-518 workflows. Consider extending theexclude
list to also omit common directories like.git
,__pycache__
,build
,dist
, and any Django migration folders to streamline lint checks.AUTHORS.md (1)
9-9
: Add new contributor entry
Welcome Gary Reynolds. For Markdown clarity, remove the backslashes escaping the angle brackets so the entry reads:- Gary Reynolds <[email protected]>
HISTORY.md (1)
3-6
: Document the PEP-518 migration under "Unreleased"
Good to track the shift to PEP-518 anduv
. Remember to relocate and timestamp this entry under the next release version when cutting the actual release.docs/conf.py (1)
26-26
: Remove unused direct module import
import django_datadog_logger
is no longer used for version retrieval. You can safely delete this import to reduce confusion.🧰 Tools
🪛 Ruff (0.11.9)
26-26:
django_datadog_logger
imported but unusedRemove unused import:
django_datadog_logger
(F401)
CONTRIBUTING.md (1)
54-60
: Format URL step as a proper list/code block
The standalone URL under step 1 isn’t clearly styled. Consider indenting it under the numbered step or wrapping it in a code block to match the style of the subsequent shell snippets.tox.ini (1)
24-29
: Unify flake8 runner configuration
You’ve set bothdependency_groups = pep8
(which pulls in flake8 via pyproject) and a standalonedeps = flake8
. To avoid duplicate installs, drop the extradeps
line and rely solely on thepep8
group.pyproject.toml (1)
53-56
: Broaden Black target versions
The[tool.black]
configuration currently targets onlypy37
andpy38
, but the project supports up to 3.13. Please updatetarget-version
to cover all supported Python versions.Makefile (3)
24-27
: Strong adoption ofuv
runner; consider DRY improvements
You’ve updated theBROWSER
andhelp
targets to useuv run python -c
, which aligns perfectly with the new PEP-518 workflow. To reduce repetition and simplify future updates, consider defining aUV := uv run
variable and then referencing$(UV)
throughout.Example diff:
+# Define uv runner for DRY usage +UV := uv run -BROWSER := uv run python -c "$$BROWSER_PYSCRIPT" +BROWSER := $(UV) python -c "$$BROWSER_PYSCRIPT"
60-63
: Coverage target modernization
The coverage commands have been ported to useuv run
, and the HTML report is automatically opened with the configuredBROWSER
. This streamlines the workflow. Consider caching the.coverage
data in CI between runs if build times become a concern.
68-74
: Sphinx docs and live-reload targets updated
Switchingsphinx-apidoc
andwatchmedo
to run underuv
maintains consistency across your tooling. For faster incremental builds, you might consider caching the Sphinx virtual environment or build artifacts in CI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lock
is excluded by!**/*.lock
📒 Files selected for processing (18)
.flake8
(1 hunks).gitignore
(0 hunks).python-version
(1 hunks)AUTHORS.md
(1 hunks)CONTRIBUTING.md
(2 hunks)HISTORY.md
(1 hunks)MANIFEST.in
(0 hunks)Makefile
(2 hunks)django_datadog_logger/__init__.py
(0 hunks)docs/.gitignore
(1 hunks)docs/Makefile
(1 hunks)docs/conf.py
(2 hunks)pyproject.toml
(1 hunks)requirements.txt
(0 hunks)requirements_dev.txt
(0 hunks)setup.cfg
(0 hunks)setup.py
(0 hunks)tox.ini
(1 hunks)
💤 Files with no reviewable changes (7)
- django_datadog_logger/init.py
- .gitignore
- requirements_dev.txt
- requirements.txt
- MANIFEST.in
- setup.cfg
- setup.py
🔇 Additional comments (8)
.python-version (1)
1-1
: Pin local Python version to 3.12
Specifying3.12
here matches the tox environment. If you intend to standardize on both 3.12 and 3.13 for local development, consider updating this to3.13
or documenting the discrepancy.docs/Makefile (1)
6-6
: Useuv
to invoke Sphinx—consistent update
Switching touv run -m sphinx
aligns with the new tooling. Ensure thatuv
is installed in your documentation CI environment and that any other Makefile targets invoking Python are similarly updated.docs/.gitignore (1)
1-3
: Looks good, doc artifacts appropriately ignored.
The new patterns will correctly prevent generated Sphinx.rst
files from being committed.CONTRIBUTING.md (1)
75-80
: Align testing instructions with updated workflow
You referencemake test-all
, but the Makefile now wraps commands withuv run
. Please confirm thatmake test-all
remains available and correctly callsuv run tox
, or update these instructions touv run tox
directly.pyproject.toml (1)
31-36
: Validate package name casing
The dependency"JSON-log-formatter"
uses uppercase; PyPI names are normalized lowercase. Confirm this matches the actual package name—if not, switch to"json-log-formatter"
.Makefile (3)
29-29
: Clean target covers all artifact types
Theclean
rule now invokesclean-test
,clean-build
, andclean-pyc
in sequence to remove test, build, coverage, and byte-compiled artifacts. This comprehensive cleanup looks solid.
51-57
: Unified linting and testing viauv run
All lint (flake8
), quick test, and full-matrix test targets now call throughuv run
, ensuring consistent environment resolution. Please verify thattox
profiles (especially for Django tests) correctly pick upDJANGO_SETTINGS_MODULE
when invoked viauv run
.
77-84
: Publishing and installation aligned withuv
Replacingtwine upload
andsetup.py
commands withuv publish
,uv build
, anduv sync --frozen
adheres to the PEP-518 build backend. Please verify in your CI environment thatuv publish
handles credentials, signing, and metadata correctly.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pyproject.toml (1)
30-35
: Includeimportlib-metadata
for Python < 3.8
The code usesimportlib.metadata
under the hood, but the backport isn’t declared in the runtime dependencies.dependencies = [ + "importlib-metadata; python_version<'3.8'", "backports.zoneinfo; python_version<'3.9'", "Django", "djangorestframework", "JSON-log-formatter", ]
🧹 Nitpick comments (2)
pyproject.toml (2)
31-35
: Pin minimal version constraints for core dependencies
Unbounded"Django"
,"djangorestframework"
, and"JSON-log-formatter"
may introduce breakages when new major versions are released. Consider specifying minimal and upper-bound versions, for example:"Django>=3.2,<5.0", "djangorestframework>=3.12,<4.0", "JSON-log-formatter>=0.4,<1.0",
64-67
: Align Black’starget-version
with supported Python versions
Black is currently targeting onlypy37
andpy38
, yet the project classifiers include 3.9–3.13. Either expandtarget-version
or remove it to default to all supported interpreters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lock
is excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml
(1 hunks)
🔇 Additional comments (1)
pyproject.toml (1)
40-46
:uv
tool not declared despite summary claiming its use
The PR description references a switch touv
, but there’s nouv
entry in dependency-groups or a[tool.uv]
section. Please clarify howuv
is intended to be installed or configured.Likely an incorrect or invalid review 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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pyproject.toml (1)
30-36
: Runtime dependencies: Consider version constraints.It's recommended to add explicit version ranges for
Django
,djangorestframework
, andJSON-log-formatter
to avoid unintended breakages on upstream releases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Makefile
(2 hunks)pyproject.toml
(1 hunks)tox.ini
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
🧰 Additional context used
🧠 Learnings (1)
tox.ini (1)
Learnt from: goodtune
PR: namespace-ee/django-datadog-logger#42
File: tox.ini:19-23
Timestamp: 2025-06-07T05:44:02.382Z
Learning: When using tox-uv with the uv-venv-lock-runner (specified as `runner = uv-venv-lock-runner` in tox.ini), dependencies are automatically installed from the uv.lock file, so explicit `deps` declarations in the [testenv] section are not required.
🔇 Additional comments (11)
tox.ini (4)
8-13
: Expanded test matrix for new Python versions: Approve.The envlist now covers Python 3.12, 3.13 and multiple PyPy interpreters, aligning with the supported versions in
pyproject.toml
. This will help catch compatibility issues early.
15-17
: Add tox-uv requirement at the top-level: Valid.Requiring
tox-uv
in the[tox]
section is necessary to activate theuv-venv-lock-runner
plugin.
19-22
: Use uv-venv-lock-runner and updated test command: Good update.Switching to
runner = uv-venv-lock-runner
and invokingdjango-admin test
directly simplifies the workflow and leverages dependencies fromuv.lock
.
24-26
: Refactored flake8 environment: Approve.Using
dependency_groups = pep8
and targeting specific paths is consistent with the new.flake8
setup.pyproject.toml (7)
1-3
: PEP 518 compliance: build-system section is correct.The
requires
andbuild-backend
entries ensure compatibility with PEP 517 frontends.
5-13
: Project metadata: Well-defined.Package name, description, README linkage, Python requirement, license, and author info are accurately declared.
14-29
: Keywords and classifiers: Comprehensive coverage.The keywords, development status, audience, and supported Python & implementation classifiers match the intended compatibility and project status.
41-52
: Dependency groups: Docs and dev correctly set up.The
docs
group includes the necessary Sphinx and parser versions and the backport forimportlib-metadata
on older Pythons; thedev
group covers essential tooling.
53-57
: PEP8 tooling group: Adequate.Including
black
,flake8
, andisort
covers formatting and style enforcement.
59-63
: setuptools configuration: Correct.Specifying packages, disabling zip safety, and including package data aligns with the previous setup.
66-68
: Black configuration: Aligned with new line length and Python targets.
line-length = 120
and updatedtarget-version
entries ensure consistency across code formatting tools.
Everything is working well locally, will need to see what happens in GitHub Actions and likely will need to make some tune-ups.
I made this to modernise the codebase, as I have another contribution I'd like to make and falling back to old tools was making life harder.
Summary by CodeRabbit
pyproject.toml
and removed legacy setup files (setup.py
,setup.cfg
,requirements.txt
, etc.)..gitignore
to track.python-version
and added ignore rules for generated documentation files..python-version
specifying Python 3.12.AUTHORS.md
.uv
tool for development tasks.