-
Notifications
You must be signed in to change notification settings - Fork 52
Improved README.rst #1251
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
Improved README.rst #1251
Conversation
WalkthroughThis pull request substantially rewrites the README documentation, reorganizing content structure, modernizing examples, and clarifying terminology around fixtures, configuration, and usage patterns. The changes consolidate verbose explanations into structured sections with concrete code examples, whilst refining guidance on installation, fixture types, customization, and configuration resolution. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @newsfragments/+3cf1375f.docs.rst:
- Around line 1-7: The newsfragment filename does not follow the required
“[issue_number].[type].rst” convention; rename the file from
"+3cf1375f.docs.rst" to "1251.docs.rst" so it matches the PR/issue-based naming;
ensure any references to the old filename in the repo (CI configs or changelog
aggregation scripts) are updated to use "1251.docs.rst" and commit the rename so
the changelog tooling picks up the fragment.
🧹 Nitpick comments (1)
README.rst (1)
152-172: Configuration section: Comprehensive coverage with one clarity concern.The configuration resolution order (lines 152–156) is clearly documented. However, line 172 in the configuration table shows the executable default as "
pg_config --bindir+pg_ctl", which is somewhat cryptic in table format. The clarifying note at lines 246–248 improves this, but consider whether the table entry itself could be more explicit (e.g., "Auto-detected viapg_config").Consider updating line 172 in the table to be more explicit about the discovery mechanism, or cross-reference the note at line 248 more prominently.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.rstnewsfragments/+3cf1375f.docs.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: postgresql_18 / postgres (pypy-3.11)
- GitHub Check: postgresql_18 / postgres (3.13)
🔇 Additional comments (10)
README.rst (10)
27-56: Quick Start section: Clear and well-structured.The introduction and Quick Start section provide excellent clarity for new users. The code example is concrete and immediately useful. The note at lines 41–43 effectively clarifies the psycopg version requirements (plugin vs. application).
71-84: Fixture type descriptions are clear and comprehensive.The distinction between Client Fixtures and Process Fixtures is well-explained, with appropriate scope and purpose information for each. The descriptions accurately reflect typical pytest fixture lifecycles.
85-104: Customizing Fixtures section: Practical and well-presented.The factory-based fixture examples are concrete and demonstrate the expected usage patterns clearly. The note at line 101–103 appropriately highlights the independent configuration of each process fixture.
108-136: Pre-population explanation: Excellent clarity on template database concept.The distinction between per-test and per-session pre-population strategies is clear. The template database cloning approach is well-explained at line 119, and the note about performance benefits is helpful. The supported parameter formats (SQL files and callable functions) are well-documented, including support for import strings.
225-225: Configuration table note requires clarification.Line 225 notes "yes (handles xdist)" for the noproc process fixture's dbname option. This notation is somewhat cryptic and could benefit from a brief explanation. Does this mean xdist-compatible naming is automatically applied, or is it informational?
Please clarify what "handles xdist" means in this context. Consider either expanding the note inline or cross-referencing any xdist-related documentation.
246-248: Executable discovery explanation: Clear and helpful.The note effectively clarifies the fallback mechanism for discovering the PostgreSQL binary when the executable is not explicitly provided. This provides important context for users in non-standard environments.
253-286: SQLAlchemy example: Syntactically correct and modern.The example demonstrates proper use of psycopg Connection attributes (lines 270–273), the correct modern connection string format for psycopg 3 (line 275:
postgresql+psycopg://), and appropriate session management with cleanup. The NullPool choice is well-suited for test fixtures.
288-315: DatabaseJanitor example: Well-documented advanced API.The example clearly demonstrates manual database state management outside standard fixtures. The comment at line 291 about Warehouse project usage provides useful real-world context.
317-343: Docker integration example: Clear and complete.The progression from Docker container setup (lines 322–324) through test configuration (lines 330–333) to test execution (lines 341–343) is logical and practical. The example effectively demonstrates the noproc fixture pattern for external PostgreSQL instances.
345-369: Database state example: Excellent demonstration of template database pattern.The example clearly shows the pre-population pattern, from load function definition through fixture creation to test usage. The explanation at lines 369 reinforces why this approach is superior: "fast, clean, and ensures no dangling transactions". This section effectively ties together the template database concept introduced earlier.
Chore that needs to be done:
pipenv run towncrier create [issue_number].[type].rstTypes are defined in the pyproject.toml, issue_number either from issue tracker or the Pull request number
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.