Reviewing a Pull Request¶
This guide explains the day-to-day steps for reviewing a PR on GitHub, using both Git and the GitHub CLI (gh
). It assumes you have already completed the one-time setup from the main workflow guide.
1. Prerequisites¶
You should already have:
- A local clone of the forked repository, with
origin
pointing to your fork andupstream
pointing to the canonical repo. - The GitHub CLI (
gh
) installed and authenticated. - Your
main
branch up to date with upstream:
2. Fetching & Checking Out the PR¶
2.1 Using GitHub CLI¶
This automatically fetches the PR and switches to a branch named
pr-<PR-number>
.
2.2 Using Plain Git¶
3. Smoke-Testing the Changes¶
Before you read code or leave comments, always verify the PR builds and tests cleanly.
3.1 Local Build¶
3.2 Container Build and testing with Postgres and Redis (compose)¶
make docker-prod # Build a new image
# Change: image: mcpgateway/mcpgateway:latest in docker-compose.yml to use the local image
make compose-up # spins up the Docker Compose stack
# Test the basics
curl -k https://localhost:4444/health` # {"status":"healthy"}
export MCPGATEWAY_BEARER_TOKEN=$(python3 -m mcpgateway.utils.create_jwt_token --username admin --exp 0 --secret my-test-key)
curl -sk -H "Authorization: Bearer $MCPGATEWAY_BEARER_TOKEN" http://localhost:4444/version | jq -c '.database, .redis'
# Add an MCP server to http://localhost:4444 then check logs:
make compose-logs
3.3 Automated Tests¶
3.4 Lint & Static Analysis¶
If any step fails, request changes and paste the relevant error logs.
4. Functional & Code Review Checklist¶
Use this checklist as you browse the changes:
Check | Why it matters |
---|---|
Does it build locally? | Ensures no missing dependencies or compile errors. |
Does it build in Docker? | Catches environment-specific issues. |
Tests are green? | Guards against regressions. |
No new lint errors? | Maintains code quality and consistency. |
Commits are clean & signed? | One-commit history & DCO compliance. |
Code follows style guidelines | Consistency in formatting, naming, and patterns. |
Security checks passed | No secrets leaked, inputs validated, etc. |
Docs / comments updated? | Documentation stays in sync with code. |
Edge cases & error handling | Robustness against invalid inputs or failures. |
5. Leaving Feedback¶
5.1 Inline Comments¶
Use gh pr review
to leave comments:
# To comment without approving
gh pr review --comment --body "Nit: rename this variable for clarity."
# To request changes
gh pr review --request-changes --body "Tests are failing on CI, please fix."
# To approve
gh pr review --approve --body "Looks good to me!"
5.2 Approving in the UI¶
- On the PR page, click "Files changed".
- Hover over a line and click the + to leave an inline comment.
- After addressing all comments, click Review changes β Approve.
6. Merging the PR (as a Maintainer)¶
Only merge once all approvals, status checks, and CI jobs are green.
- On GitHub, click Merge pull request.
- Choose Squash and merge (default) or Rebase and merge.
- Verify the commit title and body follow Conventional Commits.
- Confirm the Signed-off-by trailer is present.
- Click Confirm merge.
GitHub will delete the pr-<number>
branch automatically.
7. Cleaning Up Locally¶
After the PR is merged: * Switch back to the main branch * Delete the local feature branch * Prune deleted remote branches
git switch main
git branch -D pr-<PR-number> # replace <PR-number> with your branch name
git fetch -p # prune deleted remotes