Skip to content

Commit b56df16

Browse files
sudo-anshulsudo-anshul
authored
docs: add PR review guidelines for community reviewers (#2271)
Signed-off-by: Anshul Kushwaha <137977998+sudo-anshul@users.noreply.github.com>
1 parent 0ce2755 commit b56df16

1 file changed

Lines changed: 205 additions & 0 deletions

File tree

‎docs/sdk_developers/pr_review_guidelines.md‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
# PR Review Guidelines
2+
3+
This guide provides best practices for reviewing pull requests in the Hiero Python SDK.
4+
Whether you are a first-time reviewer or a seasoned contributor, these guidelines will help you deliver constructive, high-quality reviews that strengthen the codebase and support fellow contributors.
5+
6+
## Table of Contents
7+
8+
- [Introduction & Community Reviews](#1-introduction--community-reviews)
9+
- [Quality Checks](#2-quality-checks)
10+
- [Commit Signing](#commit-signing)
11+
- [Issue Linkage](#issue-linkage)
12+
- [Issue Resolution](#issue-resolution)
13+
- [CI Workflow Checks](#ci-workflow-checks)
14+
- [Review Best Practices](#3-review-best-practices)
15+
- [Circle of Competence](#circle-of-competence)
16+
- [Communication Style](#communication-style)
17+
- [Research Process](#research-process)
18+
- [Language and Documentation](#language-and-documentation)
19+
- [AI Review Caution](#ai-review-caution)
20+
- [Quick Reference Checklist](#4-quick-reference-checklist)
21+
- [Related Documentation & Help](#5-related-documentation--help)
22+
23+
---
24+
25+
## 1. Introduction & Community Reviews
26+
27+
Anyone from the community is welcome to review pull requests that carry the **`reviewer: community`** label. These are pull requests that maintainers have marked as ready for community feedback.
28+
29+
👉 **[View open PRs available for community review](https://github.com/hiero-ledger/hiero-sdk-python/pulls?q=is%3Aopen+is%3Apr+label%3A%22reviewer%3A+community%22)**
30+
31+
Community reviews are valuable because they:
32+
- Bring diverse perspectives and catch issues maintainers might miss
33+
- Help contributors receive faster feedback
34+
- Give reviewers the opportunity to learn from real-world code changes
35+
- Strengthen the overall quality of the SDK
36+
37+
You do not need special permissions to leave a review. Simply open the pull request, examine the changes, and share your feedback using GitHub's review tools.
38+
39+
---
40+
41+
## 2. Quality Checks
42+
43+
Before diving into the code, verify that the pull request meets the following quality criteria.
44+
45+
### Commit Signing
46+
47+
Every commit in a pull request must be signed in two ways:
48+
49+
1. **GPG Signed** — Look for the green **"Verified"** badge next to each commit on the PR's "Commits" tab. If any commit shows "Unverified," the author needs to re-sign.
50+
51+
2. **DCO Signed** — Each commit message must include a `Signed-off-by` line with the author's name and email. This is added automatically with the `-s` flag:
52+
```bash
53+
git commit -S -s -m "feat: add new feature"
54+
```
55+
56+
If commits are missing signatures, point the contributor to the [Signing Requirements](training/workflow/07_signing_requirements.md) guide or the detailed [Signing Guide](https://github.com/hiero-ledger/sdk-collaboration-hub/blob/main/guides/issue-progression/for-developers/signing.md).
57+
58+
### Issue Linkage
59+
60+
Check that the pull request description links to the issue it resolves. Look for one of these keywords followed by an issue number:
61+
62+
- `Closes #1234`
63+
- `Fixes #1234`
64+
- `Resolves #1234`
65+
66+
If the link is missing, ask the contributor to add it. Proper linkage ensures the issue is automatically closed when the PR is merged.
67+
68+
### Issue Resolution
69+
70+
Read the linked issue carefully and verify that the pull request:
71+
72+
- Addresses what the issue actually asks for — nothing more, nothing less
73+
- Meets any acceptance criteria listed in the issue description
74+
- Does not introduce unrelated changes or scope creep
75+
76+
If you are unsure whether the PR fully resolves the issue, leave a comment explaining what you think is missing or needs clarification.
77+
78+
### CI Workflow Checks
79+
80+
The repository runs automated checks on every pull request. Review the status of these checks at the bottom of the PR page.
81+
82+
**Primary checks:**
83+
84+
| Check | What It Verifies |
85+
|-------|-----------------|
86+
| Code Coverage | Test coverage meets minimum thresholds |
87+
| CodeQL | No security vulnerabilities detected |
88+
| Broken Markdown Links | All documentation links are valid |
89+
| Test File Naming | Test files follow the naming convention |
90+
91+
**Secondary checks:**
92+
93+
| Check | What It Verifies |
94+
|-------|-----------------|
95+
| Unit / Integration Tests | All tests pass across Python 3.10–3.14 |
96+
| Dependency Compatibility | No dependency conflicts |
97+
| Examples | Example scripts run without errors |
98+
| TCK Tests | Technology Compatibility Kit passes |
99+
100+
If a check fails:
101+
- Review the workflow logs to understand the failure
102+
- If the failure is in the contributor's code, point them toward the relevant guide (e.g., [Testing Guide](testing.md) or [Linting Guide](ruff.md))
103+
- If the failure appears unrelated to the PR (e.g., a flaky upstream test), note it in your review
104+
105+
---
106+
107+
## 3. Review Best Practices
108+
109+
### Circle of Competence
110+
111+
Focus your review on areas you understand well, but also stretch yourself:
112+
113+
- If you are comfortable with Python but unfamiliar with gRPC or protobuf, you can still review code style, logic flow, and test coverage
114+
- If a change touches something you do not understand, say so honestly rather than guessing
115+
- Reviewing outside your comfort zone is a great way to learn — just be transparent about your level of confidence
116+
117+
### Communication Style
118+
119+
How you communicate matters as much as what you say:
120+
121+
- **Frame uncertain feedback as questions.** Instead of "This is wrong," try "Could this cause an issue if the input is None?" Contributors may take directives literally without evaluating them critically, so questions encourage them to think.
122+
- **Be specific and constructive.** Instead of "This needs improvement," explain what exactly should change and why.
123+
- **Acknowledge good work.** If something is well-written or cleverly solved, say so. Positive feedback motivates contributors.
124+
- **Stay respectful.** Disagreement is fine; personal criticism is not. Review our [Code of Conduct](code_of_conduct.md) for community standards.
125+
126+
### Research Process
127+
128+
A thorough review requires preparation:
129+
130+
1. **Read the full issue description first.** Understand what problem the PR is solving before looking at any code.
131+
2. **Study the submitted code carefully.** Do not skim — read each changed file and understand the logic.
132+
3. **Check the surrounding context.** Click "Expand" on unchanged lines in the diff to see how new code fits into the existing file.
133+
4. **For complex changes, clone and test locally.** Open the branch in GitHub Codespaces or check it out locally to step through the code:
134+
```bash
135+
git fetch origin pull/<PR_NUMBER>/head:pr-<PR_NUMBER>
136+
git checkout pr-<PR_NUMBER>
137+
uv run pytest tests/unit/ -v
138+
```
139+
140+
### Language and Documentation
141+
142+
Code quality is not just about logic:
143+
144+
- Check for typos and grammar issues in comments, docstrings, and documentation files
145+
- Verify that new or changed functionality has appropriate documentation
146+
- Ensure variable names, function names, and class names are clear and follow existing conventions
147+
- Check that error messages are helpful and user-friendly
148+
149+
### AI Review Caution
150+
151+
AI-assisted review tools can be helpful but require critical thinking:
152+
153+
- ⚠️ **Do not accept AI-generated suggestions without verifying them.** AI tools may not understand repo-specific patterns, protobuf schemas, or SDK conventions.
154+
- ⚠️ **Verify AI suggestions against the actual codebase.** Check whether a suggested pattern actually matches how things are done in this repository.
155+
- ⚠️ **Think before suggesting AI-generated code to a contributor.** If you pass along an incorrect AI suggestion, the contributor may implement it without questioning it.
156+
157+
---
158+
159+
## 4. Quick Reference Checklist
160+
161+
### DO ✅
162+
163+
- **Read the linked issue** before reviewing any code
164+
- **Verify commit signing** — both GPG ("Verified" badge) and DCO (`Signed-off-by`)
165+
- **Check that all CI workflow checks pass** before approving
166+
- **Frame uncertain feedback as questions** to encourage critical thinking
167+
- **Be specific** — reference exact lines, suggest concrete alternatives
168+
- **Acknowledge good work** — positive feedback matters
169+
- **Test complex changes locally** or in GitHub Codespaces when possible
170+
- **Check for typos, grammar, and naming conventions** in code and docs
171+
- **Stay within your circle of competence** and be transparent about your confidence level
172+
- **Cross-reference the codebase** before suggesting patterns or approaches
173+
174+
### DON'T ❌
175+
176+
- **Don't approve without reviewing** — every approval carries weight
177+
- **Don't accept AI suggestions blindly** — verify against actual repo patterns first
178+
- **Don't make demands when unsure** — ask questions instead
179+
- **Don't review only the diff** — check surrounding context for integration issues
180+
- **Don't ignore failing CI checks** — investigate and comment on failures
181+
- **Don't leave vague feedback** like "this needs work" without specifics
182+
- **Don't make personal criticisms** — review the code, not the person
183+
- **Don't skip the issue description** — it defines what "done" looks like
184+
185+
---
186+
187+
## 5. Related Documentation & Help
188+
189+
### Related Documentation
190+
191+
| Guide | Description |
192+
|-------|-------------|
193+
| [Contribution Workflow](workflow.md) | Branching, committing, and PR process |
194+
| [Testing Guide](testing.md) | Writing and running unit and integration tests |
195+
| [Code of Conduct](code_of_conduct.md) | Community behavior standards |
196+
| [Contributing Guide](../../CONTRIBUTING.md) | Main entry point for contributors |
197+
| [Submitting a Pull Request](training/workflow/10_submit_pull_request.md) | Step-by-step PR submission details |
198+
| [Signing Requirements](training/workflow/07_signing_requirements.md) | GPG and DCO commit signing |
199+
| [Linting Guide](ruff.md) | Code quality with Ruff |
200+
201+
### Need Help?
202+
203+
- **Questions about a review?** Comment directly on the pull request — maintainers and other contributors will respond.
204+
- **General questions?** Ask on the [Linux Foundation Decentralized Trust Discord](https://discord.gg/hyperledger) (or go directly to the [#hiero-python-sdk channel](https://discord.com/channels/905194001349627914/1336494517544681563)).
205+
- **Live support?** Join the [Python SDK Office Hours](https://zoom-lfx.platform.linuxfoundation.org/meeting/99912667426?password=5b584a0e-1ed7-49d3-b2fc-dc5ddc888338) or [Community Calls](https://zoom-lfx.platform.linuxfoundation.org/meeting/92041330205?password=2f345bee-0c14-4dd5-9883-06fbc9c60581).

0 commit comments

Comments
 (0)