-
Notifications
You must be signed in to change notification settings - Fork 52
Display architecture diagram in README - closes #1244 #1249
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
Conversation
WalkthroughConfiguration and documentation updates: added an SVG exclusion to a pre-commit hook, inserted an architecture diagram and a new "How does it work" section in the README, and updated the docs changelog fragment to reference the diagram. No code or public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ 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). (1)
🔇 Additional comments (3)
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 @README.rst:
- Line 35: Update the MIN_SUPPORTED_VERSION constant in executor.py from
parse("10") to parse("14") so the version validation aligns with the README;
locate the MIN_SUPPORTED_VERSION definition in pytest-postgresql's executor
module and replace the parsed version string to "14" to enforce the >=14
requirement in the validator/logic.
🧹 Nitpick comments (1)
newsfragments/1244.docs.rst (1)
1-1: Consider adjusting the possessive form.The phrase "plugins' architecture diagram" uses a plural possessive, suggesting multiple plugins. Since this is about the pytest-postgresql plugin specifically, consider using the singular possessive "plugin's" or rephrasing to "Add architecture diagram for the plugin and display it in project's README."
🔎 Suggested correction
-Add plugins' architecture diagram and display it in project's README. +Add plugin's architecture diagram and display it in project's README.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/images/architecture.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
.pre-commit-config.yamlREADME.rstnewsfragments/1244.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). (6)
- GitHub Check: postgresql_18 / postgres (pypy-3.11)
- GitHub Check: postgresql_18 / postgres (3.11)
- GitHub Check: postgresql_18 / postgres (3.14)
- GitHub Check: postgresql_18 / postgres (3.12)
- GitHub Check: postgresql_18 / postgres (3.10)
- GitHub Check: postgresql_18 / postgres (3.13)
🔇 Additional comments (4)
.pre-commit-config.yaml (1)
18-19: LGTM! Appropriate exclusion for SVG files.Excluding SVG files from the end-of-file-fixer is sensible, as SVG is an XML-based format that may have specific formatting requirements. This change appropriately supports the addition of the architecture diagram.
README.rst (3)
1-1: LGTM! Branch reference updated appropriately.The logo URL has been correctly updated to reference the
mainbranch instead ofmaster, aligning with modern Git conventions.
55-68: LGTM! Clear documentation of plugin fixtures.The fixture descriptions provide helpful context for understanding the architecture diagram, clearly explaining the purpose and scope of each fixture type.
47-52: Architecture diagram file is present.The architecture diagram at
docs/images/architecture.svgexists in the repository and is correctly referenced in the README. No action required.
This reverts commit f330664.
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.rst (1)
35-35:⚠️ Update outdated PostgreSQL version requirement in warning.The warning states PostgreSQL versions >= 10 are tested, but according to the previous review and CI configuration, only versions >= 14 are actively tested. This should be updated to match your actual test matrix and the MIN_SUPPORTED_VERSION constant to maintain consistency across documentation, code, and CI.
🔎 Suggested fix for outdated version warning
- Tested on PostgreSQL versions >= 10. See tests for more details. + Tested on PostgreSQL versions >= 14. See tests for more details.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/images/architecture.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
README.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). (1)
- GitHub Check: postgresql_18 / postgres (pypy-3.11)
🔇 Additional comments (3)
README.rst (3)
1-3: ✅ Logo URL correctly updated from master to main.The change aligns with GitHub's default branch naming convention update. This ensures the README displays the logo from the correct branch.
55-68: Fixture descriptions are clear and well-organised.The three core fixtures (
postgresql,postgresql_proc,postgresql_noproc) are explained concisely with their intended use cases. The descriptions help users understand the scope and purpose of each fixture.
47-53: Architecture diagram is properly configured.The reStructuredText formatting is correct with proper image directive syntax, alt text, and centre alignment. The referenced SVG file exists at
docs/images/architecture.svg, and the SVG exclusion is correctly configured in.pre-commit-config.yamlwithexclude_types: [svg].
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
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.