feat(opinionated-setup): production-tested improvements

[wlritchi]

Feedback Source

Skill: opinionated-setup:setup-python-project

Summary

Improvements based on comparison with a real-world production Python project (.wlrenv),
incorporating patterns that proved useful in practice.

Changes Made

pyproject.toml Template

1. Use [dependency-groups] (PEP 735) instead of [project.optional-dependencies]

   • Modern uv-native way to declare dev dependencies
   • Updated setup/CI instructions to use uv sync --group dev

2. Expanded ruff lint rules - Added comprehensive linting:

   • N (pep8-naming)
   • S (bandit security) with S101 ignored for tests
   • ANN (annotations)
   • ASYNC (async linting)
   • UP (pyupgrade)
   • T201 (print statements)

3. mypy enable_error_code - Added truthy-bool to catch async mistakes like
   if f.exists() instead of if await f.exists()

4. pyright reportImplicitOverride - Catches missing @override decorators

.pre-commit-config.yaml Template

1. Added uv-lock hook - Keeps lockfile in sync via astral-sh/uv-pre-commit

2. Added VERSION NOTE - Reminds to check for latest versions of all hook repos

3. Enhanced config structure:

   • default_install_hook_types: [pre-commit, post-checkout]
   • default_stages: [pre-commit, pre-push, manual]
   • ci: section with autoupdate_schedule: monthly

4. Improved pre-commit-hooks:

   • Exclusions for .md and .patch files on whitespace hooks
   • Added check-case-conflict, check-executables-have-shebangs, mixed-line-ending

Other Changes

• Removed emoji from install-hooks.sh for better terminal compatibility
• Updated execution instructions to check for latest pre-commit hook versions

---

Generated by skill-feedback:reporting-feedback

[claude]

Code Review: PR #5 - Production-tested Python Setup Improvements

Summary

This PR enhances the setup-python-project skill with production-tested improvements from a real-world project. The changes are well-motivated and generally improve the quality of the generated Python projects.

✅ Strengths

1. Modern Dependency Management (PEP 735)

The migration from [project.optional-dependencies] to [dependency-groups] is excellent:

• ✅ Aligns with modern uv practices and PEP 735
• ✅ Correctly updates setup instructions (uv sync --group dev)
• ✅ Includes helpful comment explaining the rationale

2. Enhanced Linting Rules

The expanded ruff configuration adds valuable linting:

• ✅ Security checks (S - bandit) with appropriate test exemptions
• ✅ Type annotations (ANN) for better type safety
• ✅ Async linting (ASYNC) to catch common async/await mistakes
• ✅ Modern Python upgrades (UP - pyupgrade)
• ✅ Code quality rules (N - naming, T201 - print statements)

3. Better Type Checking

• ✅ mypy enable_error_code = ["truthy-bool"] - Excellent addition to catch if f.exists() vs if await f.exists() mistakes
• ✅ pyright reportImplicitOverride = true - Catches missing @override decorators

4. Improved Pre-commit Configuration

• ✅ uv-lock hook prevents lockfile drift
• ✅ default_install_hook_types and default_stages provide better control
• ✅ ci: autoupdate_schedule: monthly automates dependency updates
• ✅ Additional safety hooks (check-case-conflict, check-executables-have-shebangs, mixed-line-ending)
• ✅ Smart exclusions for .md and .patch files on whitespace hooks

5. Documentation Quality

• ✅ VERSION NOTE reminder is very helpful
• ✅ Execution instructions updated to check for latest versions
• ✅ Emoji removal for better terminal compatibility

🔍 Areas for Consideration

1. Ruff Linting Strictness

Location: opinionated-setup/skills/setup-python-project/SKILL.md:75-93

The expanded ruff rules are comprehensive but may be strict for some projects:

• ANN (annotations) requires type hints on all functions - this is good for production code but might be overwhelming for quick scripts or experimental code
• T201 (print statements) flags all prints - reasonable for libraries but may need noqa comments in CLI tools

Recommendation: Consider documenting in the skill that users can adjust these rules based on their project type. Perhaps add a comment like:

# Adjust these rules based on project needs:
# - Remove ANN for less strict type annotation requirements
# - Remove T201 if building a CLI tool where prints are intentional

2. S101 Ignore Scope

Location: opinionated-setup/skills/setup-python-project/SKILL.md:92

The comment says "Allow assert in tests" but the ignore is global, not scoped to test files.

Recommendation: Consider using ruff's per-file-ignores:

[tool.ruff.lint.per-file-ignores]
"tests/**/*.py" = ["S101"]

This would only allow asserts in test files, catching inappropriate asserts in production code.

3. Pre-commit Hook Version Checking

Location: opinionated-setup/skills/setup-python-project/SKILL.md:118

The VERSION NOTE is helpful, but the execution instructions (step 2, line 408) don't provide clear guidance on HOW to check for latest versions.

Recommendation: Consider adding specific instructions:

2. **Check for latest versions** of pre-commit hook repos:
   - Visit https://github.com/astral-sh/uv-pre-commit/releases
   - Visit https://github.com/astral-sh/ruff-pre-commit/releases
   - Visit https://github.com/pre-commit/pre-commit-hooks/releases
   - Visit https://github.com/adrienverge/yamllint/releases
   - Visit https://github.com/codespell-project/codespell/releases
   - Update the rev: values in the template below with the latest tags

4. Missing Test File Updates

The PR doesn't include a test file example with the new linting rules.

Recommendation: Consider updating the tests/test_basic.py template to demonstrate compliance with the new rules (e.g., proper type annotations, no print statements).

5. Backwards Compatibility Note

Projects using this skill before the changes would have [project.optional-dependencies].

Recommendation: Consider adding a note in the SKILL.md about migrating existing projects, or clarifying that this skill is for new projects only.

🐛 Potential Issues

None Found

No bugs or security concerns identified. The changes are well-considered and safe.

⚡ Performance Considerations

• ✅ The uv-lock hook with require_serial: true is correctly configured to avoid race conditions
• ✅ Cache mounts and dependency management are already optimized in the existing templates

🔒 Security Review

• ✅ Addition of bandit security linting (S) is a security improvement
• ✅ No security vulnerabilities introduced
• ✅ The changes enhance security posture of generated projects

🧪 Test Coverage

Observation: This PR modifies a documentation/template file (SKILL.md). There are no executable tests for skills in this repository.

Recommendation: Consider whether skill templates should have integration tests that verify generated projects work correctly. This could be a future enhancement, not blocking for this PR.

📝 Documentation Quality

• ✅ Changes are well-documented with clear rationale
• ✅ PR description is comprehensive and explains each change
• ✅ Inline comments in templates explain non-obvious decisions

Final Verdict

Overall Assessment: ✅ APPROVED

This is a high-quality PR that brings production-tested improvements to the Python project setup skill. The changes are:

• Well-motivated from real-world usage
• Thoroughly documented
• Aligned with modern Python best practices
• Backwards compatible (since it only affects new projects)

Recommendations Summary

1. Consider documenting how to adjust linting rules for different project types (nice-to-have)
2. Scope S101 ignore to test files only for better security (recommended)
3. Add specific instructions for checking hook versions (nice-to-have)
4. Update test template example to demonstrate new rules (optional)

Suggested Merge Strategy

✅ Ready to merge with optional consideration of the recommendations above. None of the suggestions are blocking - they're enhancements that could be addressed in follow-up PRs if desired.

Great work incorporating real-world learnings into the skill templates! 🎉