Conversation
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This pull request streamlines the README by reorganizing documentation into a more maintainable structure. The changes reduce the main README from 371 lines to 93 lines by moving detailed setup instructions to dedicated documentation files in the docs/ directory.
Changes:
- Condensed README to focus on quick-start instructions with references to detailed docs
- Created docs/discord.md with comprehensive Discord bot setup instructions
- Created docs/database.md with database setup, migration patterns, and the expand/migrate/contract workflow
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| README.md | Streamlined to provide concise overview, prerequisites, and setup steps; removed detailed leaderboard documentation in favor of external links |
| docs/discord.md | New file documenting Discord bot application setup, permissions, scopes, and environment variables |
| docs/database.md | New file documenting database setup, migration management with Yoyo, and expand/migrate/contract pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <summary>Click here for visual.</summary> | ||
| <img width="1440" alt="Screenshot 2024-11-24 at 12 44 08 PM" src="https://github.com/user-attachments/assets/54c34b6b-c944-4ce7-96dd-e40cfe79ffb3"> | ||
| </details> | ||
| - Python 3.11+ |
There was a problem hiding this comment.
The minimum Python version specified here (3.11+) is inconsistent with pyproject.toml which specifies Python 3.10+. The codebase uses Python 3.10 features (like the | union operator in type hints) but does not require 3.11-specific features. Consider changing this to "Python 3.10+" to match pyproject.toml, or if there's a specific reason to require 3.11+, update pyproject.toml accordingly.
| <summary>Example yoyo apply session</summary> | ||
|
|
||
| ``` | ||
| $ yoyo apply . -d postgresql://user:password@localhost/clusterdev |
There was a problem hiding this comment.
The command shown in the example uses a different path (.) than the command shown above on line 20 (src/migrations). The path should be consistent. Based on the repository structure, src/migrations is the correct path. Consider updating the example to use src/migrations instead of . to avoid confusion.
| $ yoyo apply . -d postgresql://user:password@localhost/clusterdev | |
| $ yoyo apply src/migrations -d postgresql://user:password@localhost/clusterdev |
| ```bash | ||
| git clone https://github.com/gpu-mode/discord-cluster-manager.git | ||
| cd discord-cluster-manager | ||
| pip install -e . |
There was a problem hiding this comment.
The installation command pip install -e . will only install the core dependencies but not the development dependencies (ruff, pre-commit, pytest, etc.). For contributors who want to run tests or use development tools, consider adding an optional note about installing with dev dependencies: pip install -e ".[dev]" or keeping the existing approach of using requirements-dev.txt.
1 participant
No description provided.