Transition to UV#90
Open
ajwalkiewicz wants to merge 3 commits into
Open
Conversation
Reviewer's GuideThis PR fully transitions the project to the UV build and project management tool, replacing existing workflows and build scripts with UV-based implementations and updating project metadata accordingly. Entity relationship diagram for pyproject.toml dependency groupserDiagram
PROJECT ||--o{ DEPENDENCY_GROUP : has
DEPENDENCY_GROUP {
string name
string[] dependencies
}
PROJECT {
string name
string version
string description
string[] keywords
string[] classifiers
}
PROJECT ||--o{ SCRIPT : defines
SCRIPT {
string name
string entrypoint
}
Class diagram for Makefile targets using UVclassDiagram
class Makefile {
+setup()
+check-uv()
+build()
+test()
+test_all()
+clean()
+clean_venv()
+clean_build()
+clean_cache()
+format()
+check()
+type()
+docs()
}
class UV {
+sync()
+build()
+run()
}
Makefile --> UV : uses
class ruff {
+format()
+check()
}
Makefile --> ruff : uses
class mypy {
+type_check()
}
Makefile --> mypy : uses
class pytest {
+run_tests()
}
Makefile --> pytest : uses
class sphinx_build {
+build_docs()
}
Makefile --> sphinx_build : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
Blocking issues:
- An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. (link)
General comments:
- In the pypi-publish job you need to pass your PyPI token (e.g.
pypi_token: ${{ secrets.PYPI_API_TOKEN }}) to the pypa/gh-action-pypi-publish step for authentication. - Your
make setuptarget currently just runsuv sync—you should include the dev dependency group (e.g.uv sync -g dev) so that tests, linting, and type checks install correctly in CI. - Installing UV via a curl script on every build can lead to version drift; consider pinning the installer version or adding UV as a locked dependency in your project to ensure reproducible builds.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the pypi-publish job you need to pass your PyPI token (e.g. `pypi_token: ${{ secrets.PYPI_API_TOKEN }}`) to the pypa/gh-action-pypi-publish step for authentication.
- Your `make setup` target currently just runs `uv sync`—you should include the dev dependency group (e.g. `uv sync -g dev`) so that tests, linting, and type checks install correctly in CI.
- Installing UV via a curl script on every build can lead to version drift; consider pinning the installer version or adding UV as a locked dependency in your project to ensure reproducible builds.
## Individual Comments
### Comment 1
<location> `.github/workflows/release.yml:73` </location>
<code_context>
uses: pypa/gh-action-pypi-publish@release/v1
</code_context>
<issue_to_address>
**security (yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha):** An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
* Update Move from Sphinx to Mkdocs * Update Move from Sphinx to Mkdocs * Update Move from Sphinx to Mkdocs * Update Move from Sphinx to Mkdocs * Update Move from Sphinx to Mkdocs * Update Move from Sphinx to Mkdocs * Update Move from Sphinx to Mkdocs * Update documentation
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the cochar package by migrating from setup.py to a modern pyproject.toml build system with uv as the package manager. The changes include significant refactoring of the codebase to use modern Python type hints, reorganization of imports, and improvements to code structure.
Key Changes
- Migration to modern Python packaging with uv.lock and pyproject.toml
- Modernization of type hints (Dict→dict, List→list, Union→|)
- Reorganization of code structure with better separation of concerns (config.py)
- Updated docstring format from Sphinx-style to Google-style
- Removed deprecated files (setup.py, requirements.txt, upload.sh)
Reviewed Changes
Copilot reviewed 68 out of 86 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | New lock file with dependency versions - contains some suspicious future-dated versions |
| upload.sh | Removed legacy upload script |
| setup.py | Removed in favor of pyproject.toml |
| requirements.txt | Removed in favor of uv.lock |
| tests/*.py | Updated imports to use new module structure (cochar.config, cochar.cochar) |
| src/cochar/init.py | Simplified to export only public API |
| src/cochar/config.py | Refactored configuration with modern type hints |
| src/cochar/main.py | Fixed typo in function name (pars_arguments→parse_arguments) |
| src/cochar/*.py | Updated type hints and docstring formats across all source files |
| docs/* | Documentation structure updates |
Comments suppressed due to low confidence (1)
src/cochar/main.py:24
- Function renamed from
pars_argumentstoparse_arguments, but there's a typo in the old name. The new name is correct ('parse' not 'pars').
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use UV as a new project management tool
Summary by Sourcery
Transition the project to use UV for build, test, and release management and migrate packaging to PEP 621 via pyproject.toml while updating CI pipelines accordingly.
Enhancements:
Build:
CI:
Chores: