Skip to content

Commit e5e093a

Browse files
crivetimihaicrivetimihai
authored
Merge pull request #86 from IBM/update-documentation
Add github review process docs
2 parents bd12e29 + f1c56d7 commit e5e093a

File tree

4 files changed

+148
-7
lines changed

4 files changed

+148
-7
lines changed

‎.pre-commit-config.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ repos:
291291
name: ✅ Check YAML
292292
description: Checks YAML files for parseable syntax.
293293
types: [yaml]
294-
exclude: ^(docs/|charts/mcp-stack/)
294+
exclude: ^(docs/|charts/)
295295

296296
- id: check-toml
297297
name: ✅ Check TOML
@@ -315,9 +315,9 @@ repos:
315315
name: ✅ YAMLlint - YAML Linter
316316
description: A linter for YAML files.
317317
args: [-c, .yamllint]
318-
# Lint *.yml|*.yaml everywhere EXCEPT charts/mcp-stack/**
318+
# Lint *.yml|*.yaml everywhere EXCEPT charts/**
319319
files: ^.*\.(yml|yaml)$
320-
exclude: ^charts/mcp-stack/
320+
exclude: ^charts/
321321

322322
# - repo: https://github.com/igorshubovych/markdownlint-cli
323323
# rev: v0.45.0

‎docs/docs/development/.pages

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@ nav:
33
- github.md
44
- building.md
55
- documentation.md
6+
- review.md
67
- packaging.md

‎docs/docs/development/review.md

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# Reviewing a Pull Request
2+
3+
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](./github.md).
4+
5+
---
6+
7+
## 1. Prerequisites
8+
9+
You should already have:
10+
11+
- A local clone of the forked repository, with `origin` pointing to your fork and `upstream` pointing to the canonical repo.
12+
- The GitHub CLI (`gh`) installed and authenticated.
13+
- Your `main` branch up to date with upstream:
14+
15+
```bash
16+
git fetch upstream
17+
git switch main
18+
git merge --ff-only upstream/main
19+
```
20+
21+
---
22+
23+
## 2. Fetching & Checking Out the PR
24+
25+
### 2.1 Using GitHub CLI
26+
27+
```bash
28+
gh pr checkout <PR-number>
29+
```
30+
31+
> This automatically fetches the PR and switches to a branch named `pr-<PR-number>`.
32+
33+
### 2.2 Using Plain Git
34+
35+
```bash
36+
git fetch upstream pull/<PR-number>/head:pr-<PR-number>
37+
git switch pr-<PR-number>
38+
```
39+
40+
---
41+
42+
## 3. Smoke-Testing the Changes
43+
44+
Before you read code or leave comments, **always** verify the PR builds and tests cleanly.
45+
46+
### 3.1 Local Build
47+
48+
```bash
49+
make venv install install-dev serve # Install into a fresh venv, and test it runs locally
50+
```
51+
52+
### 3.2 Container Build (if applicable)
53+
54+
```bash
55+
make docker-prod # Build a new image
56+
make compose-up # spins up the Docker Compose stack
57+
```
58+
59+
### 3.3 Automated Tests
60+
61+
```bash
62+
make test # or `pytest`, `npm test`, etc.
63+
```
64+
65+
### 3.4 Lint & Static Analysis
66+
67+
```bash
68+
make lint # runs ruff, mypy, black --check, eslint, etc.
69+
```
70+
71+
> **If any step fails**, request changes and paste the relevant error logs.
72+
73+
---
74+
75+
## 4. Functional & Code Review Checklist
76+
77+
Use this checklist as you browse the changes:
78+
79+
| Check | Why it matters |
80+
| --------------------------------- | -------------------------------------------------- |
81+
| **Does it build locally?** | Ensures no missing dependencies or compile errors. |
82+
| **Does it build in Docker?** | Catches environment-specific issues. |
83+
| **Tests are green?** | Guards against regressions. |
84+
| **No new lint errors?** | Maintains code quality and consistency. |
85+
| **Commits are clean & signed?** | One-commit history & DCO compliance. |
86+
| **Code follows style guidelines** | Consistency in formatting, naming, and patterns. |
87+
| **Security checks passed** | No secrets leaked, inputs validated, etc. |
88+
| **Docs / comments updated?** | Documentation stays in sync with code. |
89+
| **Edge cases & error handling** | Robustness against invalid inputs or failures. |
90+
91+
---
92+
93+
## 5. Leaving Feedback
94+
95+
### 5.1 Inline Comments
96+
97+
Use `gh pr review` to leave comments:
98+
99+
```bash
100+
# To comment without approving
101+
gh pr review --comment --body "Nit: rename this variable for clarity."
102+
103+
# To request changes
104+
gh pr review --request-changes --body "Tests are failing on CI, please fix."
105+
106+
# To approve
107+
gh pr review --approve --body "Looks good to me!"
108+
```
109+
110+
### 5.2 Approving in the UI
111+
112+
1. On the PR page, click **"Files changed"**.
113+
2. Hover over a line and click the **+** to leave an inline comment.
114+
3. After addressing all comments, click **Review changes** → **Approve**.
115+
116+
---
117+
118+
## 6. Merging the PR (as a Maintainer)
119+
120+
> Only merge once all approvals, status checks, and CI jobs are green.
121+
122+
1. On GitHub, click **Merge pull request**.
123+
2. Choose **Squash and merge** (default) or **Rebase and merge**.
124+
3. Verify the commit title and body follow [Conventional Commits](https://www.conventionalcommits.org/).
125+
4. Confirm the **Signed-off-by** trailer is present.
126+
5. Click **Confirm merge**.
127+
128+
GitHub will delete the `pr-<number>` branch automatically.
129+
130+
---
131+
132+
## 7. Cleaning Up Locally
133+
134+
```bash
135+
git switch main
136+
git fetch -p # prune deleted remotes
137+
git branch -D pr-<PR-number>
138+
```
139+
140+
---

‎docs/docs/overview/features.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,15 @@ MCP Gateway offers a robust feature set for integrating and managing tools, serv
1111

1212
- **Multi-Transport Support**
1313
Accessible via:
14-
14+
1515
- HTTP/JSON-RPC
1616
- WebSocket (bi-directional with ping/pong)
1717
- Server-Sent Events (SSE)
1818
- stdio (for subprocess embedding)
1919

2020
- **Unified Registry**
2121
Maintains a centralized catalog of:
22-
22+
2323
- Tools (native or REST-adapted)
2424
- Prompts (Jinja2 templates with schema validation)
2525
- Resources (MIME-aware, URI-addressable)
@@ -42,7 +42,7 @@ MCP Gateway offers a robust feature set for integrating and managing tools, serv
4242
- Register tools via REST, UI, or JSON-RPC
4343
- Wrap any REST API, CLI command, or function
4444
- Supports:
45-
45+
4646
- JSON Schema validation
4747
- Concurrency limits
4848
- Rate limiting
@@ -81,7 +81,7 @@ MCP Gateway offers a robust feature set for integrating and managing tools, serv
8181
## 🖥 Admin Interface
8282

8383
- Interactive UI with full CRUD for:
84-
84+
8585
- Tools
8686
- Resources
8787
- Prompts

0 commit comments

Comments
 (0)