Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR refactors proxy configuration handling by removing the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors proxy handling by removing the Extractor wrapper for proxy sequences and updating the Client configuration to handle proxy vectors directly. Documentation and examples were updated to use tuples for proxies, and the Response drop implementation was optimized with an #[inline] attribute. Feedback focuses on removing commented-out code in examples for better clarity, correcting a docstring for pluralization, and addressing a maintenance concern regarding inconsistency in the updated set_if_some_iter_inner macro.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/source/guide/proxy.md (1)
20-21: Remove commented legacy proxy line to avoid ambiguity.Keeping both forms in-line (one commented) makes docs noisier and may imply one form is discouraged. Prefer one canonical snippet, and document alternatives in prose if needed.
🧹 Suggested doc cleanup
- proxies=(Proxy.all("http://127.0.0.1:3067"),) - # proxies=[Proxy.http("socks5h://abc:[email protected]:6152")], + proxies=(Proxy.all("http://127.0.0.1:3067"),)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/source/guide/proxy.md` around lines 20 - 21, Remove the commented legacy proxy line to avoid ambiguity: keep a single canonical example using the active assignment (proxies=(Proxy.all("http://127.0.0.1:3067"),)) and delete the commented Proxy.http example; if you need to show alternatives, add a short prose sentence below describing the alternative use of Proxy.http/Proxy.socks with credentials instead of leaving a commented code line.examples/proxy.py (1)
9-10: Clean up commented legacy proxy config in the example.This keeps the example focused and avoids mixed signals about preferred usage.
🧹 Suggested example cleanup
- # proxies=[Proxy.http("socks5h://abc:[email protected]:6152")], - proxies=(Proxy.all("http://127.0.0.1:3067"),) + proxies=(Proxy.all("http://127.0.0.1:3067"),)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/proxy.py` around lines 9 - 10, Remove the commented legacy proxy line and any leftover commented examples (the line containing Proxy.http("socks5h://abc:[email protected]:6152")) so the example only shows the current preferred usage, i.e., the proxies assignment using Proxy.all("http://127.0.0.1:3067"); update the proxies tuple (the proxies variable) to be the single, uncommented entry and ensure no stray commented proxy configurations remain in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/source/guide/proxy.md`:
- Around line 20-21: Remove the commented legacy proxy line to avoid ambiguity:
keep a single canonical example using the active assignment
(proxies=(Proxy.all("http://127.0.0.1:3067"),)) and delete the commented
Proxy.http example; if you need to show alternatives, add a short prose sentence
below describing the alternative use of Proxy.http/Proxy.socks with credentials
instead of leaving a commented code line.
In `@examples/proxy.py`:
- Around line 9-10: Remove the commented legacy proxy line and any leftover
commented examples (the line containing
Proxy.http("socks5h://abc:[email protected]:6152")) so the example only shows the
current preferred usage, i.e., the proxies assignment using
Proxy.all("http://127.0.0.1:3067"); update the proxies tuple (the proxies
variable) to be the single, uncommented entry and ensure no stray commented
proxy configurations remain in the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d282ec69-154f-4c81-80be-c1443fdf193b
📒 Files selected for processing (7)
docs/source/guide/proxy.mdexamples/proxy.pypython/wreq/wreq.pysrc/client.rssrc/client/resp/http.rssrc/extractor.rssrc/macros.rs
from: #564
Summary by CodeRabbit
Documentation
Performance
Refactor