From 78ccf912b4f3621fcae7ada78277637bd20cec5e Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Thu, 5 Mar 2026 23:00:50 -0800 Subject: [PATCH 1/2] Improve project tooling: PR review emoji tags and simplified issue templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update PR reviewer agent to use emoji-based tags from the code review emoji guide instead of P0-P3 severity levels. This aligns review comments with the team's existing conventions and makes blocking vs non-blocking findings immediately clear. Simplify GitHub issue templates (bug report, story, task) to reduce friction — keep only the essential description field and remove verbose optional sections that were rarely filled out. --- .claude/agents/pr-reviewer.md | 77 +++++++++++++++++++-------- .github/ISSUE_TEMPLATE/bug_report.yml | 58 -------------------- .github/ISSUE_TEMPLATE/story.yml | 41 -------------- .github/ISSUE_TEMPLATE/task.yml | 35 ------------ 4 files changed, 54 insertions(+), 157 deletions(-) diff --git a/.claude/agents/pr-reviewer.md b/.claude/agents/pr-reviewer.md index 0ffc4b34..c586b0b8 100644 --- a/.claude/agents/pr-reviewer.md +++ b/.claude/agents/pr-reviewer.md @@ -110,34 +110,49 @@ For each changed file, evaluate: ### 5. Classify findings -Assign each finding a severity. Use emojis from the project's +Tag each finding with an emoji from the project's [code review emoji guide](https://github.com/erikthedeveloper/code-review-emoji-guide) -(referenced in `CONTRIBUTING.md`): +(referenced in `CONTRIBUTING.md`). The emoji communicates **reviewer intent** — +whether a comment requires action, is a suggestion, or is informational. -| Severity | Emoji | Criteria | -| ------------ | ----- | ------------------------------------------------------------------ | -| P0 — Blocker | 🔧 | Must fix before merge: bugs, data loss, security, CI failures | -| P1 — High | 🔧 | Should fix: race conditions, API design issues, missing validation | -| P2 — Medium | 🤔 | Recommended: inconsistencies, test gaps, dead code | -| P3 — Low | ⛏ | Nice to have: style, minor improvements, documentation | +#### Blocking (merge cannot proceed) -Also use 👍 to highlight particularly good code or design decisions. +| Emoji | Tag | Use when | +| ----- | ---------- | ---------------------------------------------------------------- | +| 🔧 | **wrench** | A necessary change: bugs, data loss, security, missing validation, CI failures | +| ❓ | **question** | A question that must be answered before you can complete the review | + +#### Non-blocking (merge can proceed) + +| Emoji | Tag | Use when | +| ----- | ---------------- | --------------------------------------------------------------------- | +| 🤔 | **thinking** | Thinking aloud — expressing a concern or exploring alternatives | +| ♻️ | **refactor** | A concrete refactoring suggestion with enough context to act on | +| 🌱 | **seedling** | A future-focused observation — not for this PR but worth considering | +| 📝 | **note** | An explanatory comment or context — no action required | +| ⛏ | **nitpick** | A stylistic or formatting preference — does not require changes | +| 🏕 | **camp site** | An opportunity to leave the code better than you found it (boy scout rule) | +| 📌 | **out of scope** | An important concern outside this PR's scope — needs a follow-up issue | +| 👍 | **praise** | Highlight particularly good code, design, or testing decisions | ### 6. Present findings for user approval **Do not submit the review automatically.** Present all findings to the user organized by severity, with: -- Severity and title +- Emoji tag and title - File path and line number - Description and suggested fix - Whether it would be an inline comment or body-level finding +Group findings into two sections: **Blocking** (🔧 / ❓) and **Non-blocking** +(everything else). This makes it immediately clear what must be addressed. + Ask the user which findings to include in the PR review. The user may: - Approve all findings - Exclude specific findings -- Adjust severity levels +- Change emoji tags - Edit descriptions - Add additional comments @@ -149,10 +164,10 @@ After user approval, submit the selected findings as a formal review. #### Determine the review verdict -- If any P0 findings are included: `CHANGES_REQUESTED` -- If any P1 findings are included: `CHANGES_REQUESTED` -- If only P2 or below: `COMMENT` -- If no findings: `APPROVE` +- If any 🔧 (wrench) findings are included: `REQUEST_CHANGES` +- If any ❓ (question) findings are included: `COMMENT` (questions need answers, not change requests) +- If only non-blocking findings (🤔 ♻️ 🌱 📝 ⛏ 🏕 📌 👍): `COMMENT` +- If no findings (or only 👍 praise): `APPROVE` #### Build inline comments @@ -165,7 +180,7 @@ comment. Use the file's **current line number** (not diff position) with the "path": "crates/common/src/publisher.rs", "line": 166, "side": "RIGHT", - "body": "🔧 **Race condition**: Description of the issue...\n\n**Fix**:\n```rust\n// suggested code\n```" + "body": "🔧 **wrench** — Race condition: Description of the issue...\n\n**Fix**:\n```rust\n// suggested code\n```" } ```` @@ -179,24 +194,38 @@ concerns, architectural issues, dependency problems) in the review body: <1-2 sentence overview of the changes and overall assessment> -## Findings +## Blocking + +### 🔧 wrench + +- **Title**: description (file:line) + +### ❓ question -### 🔧 Blockers (P0) +- **Title**: description (file:line) + +## Non-blocking + +### 🤔 thinking - **Title**: description (file:line) -### 🔧 High (P1) +### ♻️ refactor - **Title**: description (file:line) -### 🤔 Medium (P2) +### 🌱 seedling / 🏕 camp site / 📌 out of scope - **Title**: description -### ⛏ Low (P3) +### ⛏ nitpick - **Title**: description +### 👍 praise + +- **Title**: description (file:line) + ## CI Status - fmt: PASS/FAIL @@ -205,6 +234,8 @@ concerns, architectural issues, dependency problems) in the review body: - js tests: PASS/FAIL ``` +Omit any section that has no findings — don't include empty headings. + #### Submit the review Use the GitHub API to submit. Handle these known issues: @@ -242,8 +273,8 @@ Where `comments.json` contains the array of inline comment objects. Output: - The review URL -- Total findings by severity (e.g., "2 P0, 3 P1, 5 P2, 2 P3") -- Whether the review requested changes or approved +- Total findings by category (e.g., "2 🔧, 1 ❓, 3 🤔, 2 ⛏, 1 👍") +- Whether the review requested changes, commented, or approved - Any CI failures encountered ## Rules diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 48e1d755..6e86c2ca 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -15,61 +15,3 @@ body: placeholder: What happened? validations: required: true - - - type: textarea - id: reproduction - attributes: - label: Steps to reproduce - description: Minimal steps to trigger the bug. - placeholder: | - 1. Configure trusted-server.toml with ... - 2. Run `fastly compute serve` - 3. Send a request to ... - 4. See error ... - validations: - required: true - - - type: textarea - id: expected - attributes: - label: Expected behavior - description: What should have happened instead? - validations: - required: true - - - type: dropdown - id: area - attributes: - label: Affected area - description: Which part of the project is affected? - multiple: true - options: - - Core (synthetic IDs, cookies, GDPR) - - Integrations (prebid, lockr, permutive, etc.) - - HTML processing / JS injection - - Ad serving (Equativ) - - Fastly runtime - - JS build pipeline - - CI / Tooling - validations: - required: true - - - type: input - id: version - attributes: - label: Version - description: Git SHA or version. - placeholder: "commit abc1234" - - - type: textarea - id: logs - attributes: - label: Relevant log output - description: Paste any error messages or logs. - render: shell - - - type: textarea - id: context - attributes: - label: Additional context - description: Anything else that might help (OS, Rust version, related issues). diff --git a/.github/ISSUE_TEMPLATE/story.yml b/.github/ISSUE_TEMPLATE/story.yml index a847e2cd..a91f161d 100644 --- a/.github/ISSUE_TEMPLATE/story.yml +++ b/.github/ISSUE_TEMPLATE/story.yml @@ -15,44 +15,3 @@ body: placeholder: "As a publisher, I want ... so that ..." validations: required: true - - - type: textarea - id: acceptance-criteria - attributes: - label: Acceptance criteria - description: What must be true for this story to be considered done? - placeholder: | - - [ ] Criterion 1 - - [ ] Criterion 2 - validations: - required: true - - - type: dropdown - id: scope - attributes: - label: Affected area - description: Which part of the project would this touch? - multiple: true - options: - - Core (synthetic IDs, cookies, GDPR) - - Integrations (prebid, lockr, permutive, etc.) - - HTML processing / JS injection - - Ad serving (Equativ) - - Fastly runtime - - JS build pipeline - - Documentation - - CI / Tooling - validations: - required: true - - - type: textarea - id: solution - attributes: - label: Proposed approach - description: How should this be implemented? Include API examples if relevant. - - - type: textarea - id: context - attributes: - label: Additional context - description: Links, mockups, related issues, or prior art. diff --git a/.github/ISSUE_TEMPLATE/task.yml b/.github/ISSUE_TEMPLATE/task.yml index 91a7eb99..3eec12ca 100644 --- a/.github/ISSUE_TEMPLATE/task.yml +++ b/.github/ISSUE_TEMPLATE/task.yml @@ -14,38 +14,3 @@ body: description: What needs to be done and why? validations: required: true - - - type: textarea - id: done-criteria - attributes: - label: Done when - description: How do we know this task is complete? - placeholder: | - - [ ] Criterion 1 - - [ ] Criterion 2 - validations: - required: true - - - type: dropdown - id: scope - attributes: - label: Affected area - description: Which part of the project would this touch? - multiple: true - options: - - Core (synthetic IDs, cookies, GDPR) - - Integrations (prebid, lockr, permutive, etc.) - - HTML processing / JS injection - - Ad serving (Equativ) - - Fastly runtime - - JS build pipeline - - Documentation - - CI / Tooling - validations: - required: true - - - type: textarea - id: context - attributes: - label: Additional context - description: Related issues, dependencies, or notes. From d23f8bfbf0d0cab3c96007c25bdbc1403e472218 Mon Sep 17 00:00:00 2001 From: Aram Grigoryan <132480+aram356@users.noreply.github.com> Date: Fri, 6 Mar 2026 00:26:11 -0800 Subject: [PATCH 2/2] Formatting --- .claude/agents/pr-reviewer.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/.claude/agents/pr-reviewer.md b/.claude/agents/pr-reviewer.md index c586b0b8..0a596f41 100644 --- a/.claude/agents/pr-reviewer.md +++ b/.claude/agents/pr-reviewer.md @@ -117,23 +117,23 @@ whether a comment requires action, is a suggestion, or is informational. #### Blocking (merge cannot proceed) -| Emoji | Tag | Use when | -| ----- | ---------- | ---------------------------------------------------------------- | -| 🔧 | **wrench** | A necessary change: bugs, data loss, security, missing validation, CI failures | -| ❓ | **question** | A question that must be answered before you can complete the review | +| Emoji | Tag | Use when | +| ----- | ------------ | ------------------------------------------------------------------------------ | +| 🔧 | **wrench** | A necessary change: bugs, data loss, security, missing validation, CI failures | +| ❓ | **question** | A question that must be answered before you can complete the review | #### Non-blocking (merge can proceed) -| Emoji | Tag | Use when | -| ----- | ---------------- | --------------------------------------------------------------------- | -| 🤔 | **thinking** | Thinking aloud — expressing a concern or exploring alternatives | -| ♻️ | **refactor** | A concrete refactoring suggestion with enough context to act on | -| 🌱 | **seedling** | A future-focused observation — not for this PR but worth considering | -| 📝 | **note** | An explanatory comment or context — no action required | -| ⛏ | **nitpick** | A stylistic or formatting preference — does not require changes | +| Emoji | Tag | Use when | +| ----- | ---------------- | -------------------------------------------------------------------------- | +| 🤔 | **thinking** | Thinking aloud — expressing a concern or exploring alternatives | +| ♻️ | **refactor** | A concrete refactoring suggestion with enough context to act on | +| 🌱 | **seedling** | A future-focused observation — not for this PR but worth considering | +| 📝 | **note** | An explanatory comment or context — no action required | +| ⛏ | **nitpick** | A stylistic or formatting preference — does not require changes | | 🏕 | **camp site** | An opportunity to leave the code better than you found it (boy scout rule) | -| 📌 | **out of scope** | An important concern outside this PR's scope — needs a follow-up issue | -| 👍 | **praise** | Highlight particularly good code, design, or testing decisions | +| 📌 | **out of scope** | An important concern outside this PR's scope — needs a follow-up issue | +| 👍 | **praise** | Highlight particularly good code, design, or testing decisions | ### 6. Present findings for user approval