Skip to content

Commit 620b7fb

Browse files
proyectoauraorgproyectoauraorg
committed
docs: add PR Zoo-Code-Org#194 lessons learned and update playbook pre-submission checklist
1 parent 761fb7b commit 620b7fb

2 files changed

Lines changed: 112 additions & 28 deletions

File tree

‎docs/PR-194-LESSONS-LEARNED.md‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# PR #194 — Lessons Learned
2+
3+
> Post-mortem: Upstream PR attempt for Issue #193 (diagnostics prefix fix)
4+
> Date: 2026-05-19
5+
6+
## Timeline
7+
8+
| Date | Event |
9+
| ------------- | --------------------------------------------------------------------------------------------------------------------------------- |
10+
| 2026-05-19 AM | PR #194 opened upstream targeting Issue #193 (`roo-` → `zoo-` prefix in diagnostics) |
11+
| 2026-05-19 PM | PR #194 closed as "duplicate" of PR #199 |
12+
| 2026-05-19 PM | Investigation: PR #199 is **not** a duplicate — it fixes `roo-deploy/` prefix (CompanionDeployer.ts), a completely different file |
13+
| 2026-05-19 PM | PR #16 opened on fork with clean scope (2 files only) for upstream submission |
14+
15+
## Root Cause Analysis
16+
17+
### Why PR #194 was closed incorrectly
18+
19+
PR #194 and PR #199 both address `roo-` remnants from the same Issue #193, but fix **different files**:
20+
21+
| PR | Target File | Prefix Fixed |
22+
| ---- | ----------------------- | --------------------------------------- |
23+
| #194 | `diagnosticsHandler.ts` | `roo-diagnostics-` → `zoo-diagnostics-` |
24+
| #199 | `CompanionDeployer.ts` | `roo-deploy/` → `zoo-deploy/` |
25+
26+
The upstream maintainer likely saw both PRs reference #193 and assumed they were duplicates without checking the actual file diffs.
27+
28+
### Contributing Factors
29+
30+
1. **Broad Issue Scope**: Issue #193 covers ALL `roo-` remnants across the codebase, making it possible for multiple PRs to target the same issue legitimately
31+
2. **PR Description**: Our PR #194 description could have been more explicit about the narrow file scope
32+
3. **No Cross-Reference**: Neither PR mentioned the other, making it easy to confuse them
33+
34+
## Lessons & Action Items
35+
36+
### 1. Narrow PR Scope Even on Broad Issues
37+
38+
- **Lesson**: When an issue covers multiple files/areas, always title the PR to indicate the specific file or component being fixed
39+
- **Action**: Use titles like `fix: update diagnostics prefix from roo- to zoo- (#193)` (specific) instead of `fix: update remaining roo- prefixes (#193)` (vague)
40+
- **Applied**: ✅ PR #16 uses specific title
41+
42+
### 2. Cross-Reference Existing PRs
43+
44+
- **Lesson**: Before opening a PR, check if there are other PRs targeting the same issue
45+
- **Action**: Add to pre-submission checklist: "Check for existing PRs on same issue and cross-reference in description"
46+
- **Applied**: ✅ Will be added to CONTRIBUTION-PLAYBOOK.md
47+
48+
### 3. Explicit Scope Declaration in Body
49+
50+
- **Lesson**: The PR body should explicitly state which files are changed and which are NOT
51+
- **Action**: Use "Changes (N files):" format with explicit file listing
52+
- **Applied**: ✅ PR #16 body lists exact files
53+
54+
### 4. Error Handling for Fork PRs
55+
56+
- **Lesson**: `gh pr create` with heredoc syntax fails on shell quoting issues
57+
- **Action**: Use `--body-file` with a temp file instead of inline `--body` with heredocs
58+
- **Applied**: ✅ PR #16 used `--body-file /tmp/pr-body-v2.md`
59+
60+
## PR #16 Status
61+
62+
| Check | Status |
63+
| ------------- | ---------------------------------------------------------------------------------- |
64+
| Branch | `fix/193-diagnostic-prefix-v2` |
65+
| Files changed | 2 (diagnosticsHandler.ts + spec) |
66+
| typecheck | ✅ Pass |
67+
| lint | ✅ Pass |
68+
| tests | ✅ 5/5 pass |
69+
| Fork PR | [proyectoauraorg/Zoo-Code#16](https://github.com/proyectoauraorg/Zoo-Code/pull/16) |
70+
| Upstream PR | Pending submission |

‎docs/UPSTREAM-CONTRIBUTION-PLAYBOOK.md‎

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@ Transform this fork into a **synchronized staging integration branch** aligned c
88

99
## Branch Naming Convention (OBLIGATORY)
1010

11-
| Type | Prefix | Example |
12-
|----------|------------|-----------------------------------|
13-
| Fix | `fix/` | `fix/193-diagnostic-prefix` |
14-
| Feature | `feat/` | `feat/user-agent-migration` |
15-
| Refactor | `refactor/`| `refactor/62-remove-stale-mcps` |
16-
| Docs | `docs/` | `docs/update-user-guide` |
17-
| Test | `test/` | `test/coverage-marketplace` |
18-
| Chore | `chore/` | `chore/update-deps` |
11+
| Type | Prefix | Example |
12+
| -------- | ----------- | ------------------------------- |
13+
| Fix | `fix/` | `fix/193-diagnostic-prefix` |
14+
| Feature | `feat/` | `feat/user-agent-migration` |
15+
| Refactor | `refactor/` | `refactor/62-remove-stale-mcps` |
16+
| Docs | `docs/` | `docs/update-user-guide` |
17+
| Test | `test/` | `test/coverage-marketplace` |
18+
| Chore | `chore/` | `chore/update-deps` |
1919

2020
**Critical rule**: `TYPE/ISSUE-NUMBER-brief-description`
2121

@@ -34,6 +34,7 @@ Transform this fork into a **synchronized staging integration branch** aligned c
3434
## Social Rules for Maintainers
3535

3636
### DO:
37+
3738
- Small, focused PRs
3839
- Semantic commits
3940
- Clear descriptions
@@ -42,6 +43,7 @@ Transform this fork into a **synchronized staging integration branch** aligned c
4243
- Include tests
4344

4445
### DON'T:
46+
4547
- Mega PRs
4648
- Ideological changes
4749
- Mass reformats
@@ -53,15 +55,26 @@ Transform this fork into a **synchronized staging integration branch** aligned c
5355

5456
## PR Validation Checklist (MANDATORY before submission)
5557

56-
| Validation | Required |
57-
|---------------|----------|
58-
| Typecheck | ✅ |
59-
| Lint | ✅ |
60-
| Tests | ✅ |
61-
| Coverage | ≥95% |
62-
| Build | ✅ |
63-
| Clean rebase | ✅ |
64-
| Conflicts | 0 |
58+
### Technical Validation
59+
60+
| Validation | Required |
61+
| ------------ | -------- |
62+
| Typecheck | ✅ |
63+
| Lint | ✅ |
64+
| Tests | ✅ |
65+
| Coverage | ≥95% |
66+
| Build | ✅ |
67+
| Clean rebase | ✅ |
68+
| Conflicts | 0 |
69+
70+
### Pre-Submission Intelligence
71+
72+
| Check | Required |
73+
| ---------------------------------------- | ---------------------------------- |
74+
| Existing PRs on same issue? | ✅ Cross-reference in body |
75+
| Title indicates specific file/component? | ✅ Narrow scope |
76+
| Body lists exact changed files? | ✅ Use "Changes (N files):" format |
77+
| No heredoc in `gh pr create`? | ✅ Use `--body-file` instead |
6578

6679
---
6780

@@ -122,23 +135,24 @@ Maintainer Delivery (official PR upstream)
122135

123136
## Priority Classification
124137

125-
| Priority | Type | Action |
126-
|----------|-----------------------|---------------|
127-
| P0 | CI broken | Immediate |
128-
| P1 | Reproducible bugs | High priority |
129-
| P2 | Safe refactors | Medium |
130-
| P3 | DX / docs | Normal |
131-
| P4 | Complex features | Plan first |
138+
| Priority | Type | Action |
139+
| -------- | ----------------- | ------------- |
140+
| P0 | CI broken | Immediate |
141+
| P1 | Reproducible bugs | High priority |
142+
| P2 | Safe refactors | Medium |
143+
| P3 | DX / docs | Normal |
144+
| P4 | Complex features | Plan first |
132145

133146
---
134147

135148
## Current State
136149

137150
- **Sync status**: main === upstream/main ✅
138-
- **Active PR**: #196 (refactor/62-remove-stale-organizationMcps) — REVIEW_REQUIRED
139-
- **Pending decision**: feature/font-size-setting branch
140-
- **Next action**: Issue index (FASE 2)
151+
- **Active PR**: #16 (fix/193-diagnostic-prefix-v2) — Pending upstream submission
152+
- **Fork PRs**: #15 (rebrand remnants, broad), #16 (diagnostic prefix, narrow)
153+
- **Lesson applied**: PR #194 closed as duplicate → re-submitted as PR #16 with narrow scope
154+
- **Next action**: Submit PR #16 upstream; evaluate PR #15 scope reduction
141155

142156
---
143157

144-
*Last updated: 2026-05-19T19:01 CDT*
158+
_Last updated: 2026-05-19T20:35 CDT_

0 commit comments

Comments
 (0)