-
Notifications
You must be signed in to change notification settings - Fork 1
Modernize #1
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
Conversation
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.
Pull request overview
This PR modernizes the project's build system, tooling, and codebase to align with current Python best practices. It migrates from setup.py to pyproject.toml, updates from Python 3.6/3.9 to 3.10/3.14 (though 3.14 doesn't exist yet), replaces flake8 with Ruff for linting, and converts legacy string formatting to f-strings.
Key changes:
- Migration from setup.py to pyproject.toml with setuptools-scm for versioning
- Tooling upgrade: flake8 → Ruff, removal of check-manifest and bandit from tox environments
- Code style modernization: %-formatting and .format() → f-strings, set comprehensions, tuple argument for str.endswith()
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | New build configuration with project metadata, dependencies, and Ruff configuration |
| setup.py | Deleted - replaced by pyproject.toml |
| requirements.txt | Deleted - dependencies now in pyproject.toml |
| tox.ini | Updated to use py310/py314, Ruff instead of flake8, removed docs/security/venv environments |
| .github/workflows/tests.yml | Updated Python versions to 3.10/3.14 and GitHub Actions to v6 |
| .github/workflows/release.yml | Modernized release workflow with trusted publishing and uv for building |
| src/mgit/git.py | Code style updates: f-strings, set comprehensions, tuple arguments |
| src/mgit/cli.py | Code style updates: f-strings, removed blank lines |
| src/mgit/init.py | Code style updates: f-strings, comment alignment |
| tests/test_git.py | Code style updates: quotes, formatting |
| tests/test_reporting.py | Code style updates: quotes, comment spacing |
| tests/conftest.py | Removed extra blank line |
| docs/contributing.rst | Updated setup instructions to use uv instead of tox venv |
| README.rst | Updated setup instructions to use uv instead of tox venv |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: codecov/codecov-action@v1 | ||
| with: | ||
| file: .tox/test-reports/coverage.xml | ||
| - uses: codecov/codecov-action@v5 |
Copilot
AI
Dec 5, 2025
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.
The codecov action configuration is missing the coverage file path. The previous configuration explicitly specified file: .tox/test-reports/coverage.xml, but this has been removed.
While codecov v4 can auto-discover coverage files, explicitly specifying the file path makes the configuration more robust and clear. Consider adding:
with:
file: .tox/test-reports/coverage.xml| - uses: codecov/codecov-action@v5 | |
| - uses: codecov/codecov-action@v5 | |
| with: | |
| file: .tox/test-reports/coverage.xml |
| -rtests/requirements.txt | ||
| commands = pytest {posargs:-vv --cov=src --cov-report=xml} | ||
| deps = -rtests/requirements.txt | ||
| commands = pytest {posargs:-vv --cov=src --cov=tests --cov-report= tests} |
Copilot
AI
Dec 5, 2025
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.
The test command now covers the tests directory itself (--cov=tests), which is unusual. Typically, test coverage should only measure the source code being tested, not the tests themselves.
Consider removing --cov=tests and keeping only --cov=src, or clarify if there's a specific reason to measure test coverage.
| """ | ||
| cmd = ["git"] | ||
| if args and args[0] == "clone": | ||
| args_represented = "git %s" % " ".join(args) |
Copilot
AI
Dec 5, 2025
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.
Incomplete f-string conversion. This line still uses old-style % formatting ("git %s" % ...) instead of being converted to an f-string for consistency with the other changes in this PR.
Consider changing to: f"git {' '.join(args)}" for consistency.
| args_represented = "git %s" % " ".join(args) | |
| args_represented = f"git {' '.join(args)}" |
This PR modernizes the project's build system, tooling, and codebase to align with current Python best practices.