-
-
Notifications
You must be signed in to change notification settings - Fork 219
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
Documentation: Update change log for upcoming version 3.0.0 #552
Conversation
WalkthroughThe pull request introduces significant updates to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
062771a
to
f16ed34
Compare
This test matrix is insane! Haha i like it |
8b2f5c8
to
aed0c68
Compare
aed0c68
to
04613f4
Compare
04613f4
to
92a6f8e
Compare
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.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (7)
docs/source/sandbox.md (2)
9-9
: Consider adding SSH clone option.For contributors who prefer SSH authentication, consider adding the SSH clone URL as an alternative option.
git clone https://github.com/kennethreitz/responder.git +# Or using SSH +git clone [email protected]:kennethreitz/responder.git
14-17
: Enhance documentation of installation extras.The current description doesn't clearly explain what each extra provides. Consider adding details about what's included in each category.
Install project in editable mode, including all runtime extensions and development tools. + +Available extras: +- full: All runtime extensions +- develop: Development tools (linting, formatting) +- docs: Documentation generation tools +- release: Release management tools +- test: Testing frameworks and dependencies + uv pip install --editable '.[full,develop,docs,release,test]'CHANGELOG.md (5)
10-11
: Use YYYY-MM-DD format for the release dateAccording to Keep a Changelog format, the release date should use YYYY-MM-DD format. Consider replacing
2024-11-xx
with the actual release date when known.
29-29
: Add migration guidance for API changeThe change from GET to POST is a breaking change. Consider adding migration guidance or examples to help users update their code.
53-57
: Add migration example for removed --build flagConsider adding an example showing how users should migrate from
responder run --build
to the newresponder build
command.
61-64
: Add context for fixesConsider adding more context to help users understand:
- The impact of the Windows routing fix
- Whether any configuration changes are needed for the uvicorn debug mode change
- The specific GraphQL dependency versions that are now pinned
Line range hint
391-391
: Update repository linksThe comparison links at the bottom of the changelog point to
taoufik07/responder
but should point tokennethreitz/responder
based on the PR URL.🧰 Tools
🪛 LanguageTool
[duplication] ~47-~47: Possible typo: you repeated a word
Context: ... Modernized and trimmed list of runtime dependencies - Dependencies: Switched from WhiteNoise to ServeStati...(ENGLISH_WORD_REPEAT_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
CHANGELOG.md
(1 hunks)docs/source/sandbox.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[duplication] ~47-~47: Possible typo: you repeated a word
Context: ... Modernized and trimmed list of runtime dependencies - Dependencies: Switched from WhiteNoise to ServeStati...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (1)
CHANGELOG.md (1)
12-25
: LGTM! Clear and well-documented additions
The Added section effectively communicates new features with helpful examples, particularly for the CLI changes.
cd responder | ||
python3 -m venv .venv | ||
source .venv/bin/activate | ||
uv venv |
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.
Add context for uv
tool and provide alternatives.
The uv
tool might not be available for all users. Consider:
- Adding a brief explanation of what
uv
is - Including installation instructions for
uv
- Providing alternative commands using standard tools
+# Install uv (if not already installed)
+pip install uv
+
+# Create virtual environment (choose one method):
+# Using uv (recommended for faster installation):
uv venv
+
+# Or using standard tools:
+python -m venv .venv
+source .venv/bin/activate # On Unix/macOS
+# or
+.venv\Scripts\activate # On Windows
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
uv venv | |
# Install uv (if not already installed) | |
pip install uv | |
# Create virtual environment (choose one method): | |
# Using uv (recommended for faster installation): | |
uv venv | |
# Or using standard tools: | |
python -m venv .venv | |
source .venv/bin/activate # On Unix/macOS | |
# or | |
.venv\Scripts\activate # On Windows |
source .venv/bin/activate | ||
poe check |
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.
Improve operations section documentation.
The section needs improvements for clarity and cross-platform compatibility:
- The virtual environment activation command is platform-specific
- The
poe
command is used without explanation
+# Ensure your virtual environment is activated:
+# On Unix/macOS:
source .venv/bin/activate
+# On Windows:
+.venv\Scripts\activate
+
+# Install poethepoet if not already installed:
+pip install poethepoet
+
+# Run checks (linting and tests):
poe check
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
source .venv/bin/activate | |
poe check | |
# Ensure your virtual environment is activated: | |
# On Unix/macOS: | |
source .venv/bin/activate | |
# On Windows: | |
.venv\Scripts\activate | |
# Install poethepoet if not already installed: | |
pip install poethepoet | |
# Run checks (linting and tests): | |
poe check |
- Dependencies: Modernized and trimmed list of runtime dependencies | ||
- Dependencies: Switched from WhiteNoise to ServeStatic |
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.
Fix duplicate word "Dependencies"
Combine the two bullet points to avoid repetition:
-Dependencies: Modernized and trimmed list of runtime dependencies
-Dependencies: Switched from WhiteNoise to ServeStatic
+Dependencies:
+- Modernized and trimmed list of runtime dependencies
+- Switched from WhiteNoise to ServeStatic
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- Dependencies: Modernized and trimmed list of runtime dependencies | |
- Dependencies: Switched from WhiteNoise to ServeStatic | |
Dependencies: | |
- Modernized and trimmed list of runtime dependencies | |
- Switched from WhiteNoise to ServeStatic |
🧰 Tools
🪛 LanguageTool
[duplication] ~47-~47: Possible typo: you repeated a word
Context: ... Modernized and trimmed list of runtime dependencies - Dependencies: Switched from WhiteNoise to ServeStati...
(ENGLISH_WORD_REPEAT_RULE)
... in retrospective, after summarizing all commits since v2.0.7, omitting all infrastructure-/project-/sandbox-related ones, and also omitting dependency package update chores, only listing significant ones.