feat: redo the package #4
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request implements a complete package rename from my_python_package to greeting-toolkit. The changes encompass all project files including source code, documentation, configuration files, and development scripts to establish a cohesive naming convention.
- Package renamed from
my_python_packagetogreeting_toolkit(module name) andgreeting-toolkit(distribution name) - Updated all import statements, configuration files, and documentation references
- Modified CLI command from
my-python-packagetogreeting-toolkit
Reviewed Changes
Copilot reviewed 50 out of 51 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Multiple test files | Updated import statements and test configurations for the renamed package |
| Source code files | Complete implementation of the greeting toolkit with proper module structure |
| Configuration files | Updated package references in pyproject.toml, tox.ini, setup.cfg, and other config files |
| Documentation files | Updated all documentation to reflect the new package name and usage |
| Scripts and tooling | Modified development scripts and GitHub workflows for the new package name |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| def test_random_greeting(): | ||
| """Test random greeting selection.""" | ||
| with patch("my_python_package.core.random.choice", return_value="Hi"): | ||
| with patch("greeting_toolkit.core.secrets.choice", return_value="Hi"): |
There was a problem hiding this comment.
The patch target is incorrect. The test is patching secrets.choice but the actual code in core.py uses secrets.choice directly. The patch should target greeting_toolkit.core.secrets.choice which matches the import in the core module.
| configure_logging(log_file=outside) | ||
| warn.assert_called() | ||
|
|
||
| assert not any(isinstance(h, logging.FileHandler) for h in logger.handlers) |
There was a problem hiding this comment.
Using /etc/passwd as a test path could be problematic on some systems. Consider using a more neutral absolute path like /tmp/nonexistent/test.log or a path that's guaranteed to be outside the working directory without referencing sensitive system files.
| if log_file: | ||
| try: | ||
| file_path = Path(log_file).expanduser().resolve() | ||
| cwd = Path.cwd().resolve() |
There was a problem hiding this comment.
The security check uses is_relative_to() but this method was only introduced in Python 3.9. Since the project supports Python 3.10+, this is acceptable, but consider adding a comment or ensuring the minimum Python version requirement is clearly documented.
| cwd = Path.cwd().resolve() | |
| cwd = Path.cwd().resolve() | |
| # Path.is_relative_to() requires Python 3.9+; project supports Python 3.10+. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Pull Request
Description
Related Issues
Type of change
How Has This Been Tested?
Checklist
Screenshots (if applicable)
Additional context