Skip to content

Commit 867d4e5

Browse files
committed
Merge branch 'feature/integration-testing-with-testcontainers' of github.com-prkjr:IABTechLab/trusted-server into feature/integration-testing-with-testcontainers
2 parents 012b8bd + 9340a86 commit 867d4e5

5 files changed

Lines changed: 56 additions & 159 deletions

File tree

.claude/agents/pr-reviewer.md

Lines changed: 54 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -110,34 +110,49 @@ For each changed file, evaluate:
110110

111111
### 5. Classify findings
112112

113-
Assign each finding a severity. Use emojis from the project's
113+
Tag each finding with an emoji from the project's
114114
[code review emoji guide](https://github.com/erikthedeveloper/code-review-emoji-guide)
115-
(referenced in `CONTRIBUTING.md`):
115+
(referenced in `CONTRIBUTING.md`). The emoji communicates **reviewer intent**
116+
whether a comment requires action, is a suggestion, or is informational.
116117

117-
| Severity | Emoji | Criteria |
118-
| ------------ | ----- | ------------------------------------------------------------------ |
119-
| P0 — Blocker | 🔧 | Must fix before merge: bugs, data loss, security, CI failures |
120-
| P1 — High | 🔧 | Should fix: race conditions, API design issues, missing validation |
121-
| P2 — Medium | 🤔 | Recommended: inconsistencies, test gaps, dead code |
122-
| P3 — Low || Nice to have: style, minor improvements, documentation |
118+
#### Blocking (merge cannot proceed)
123119

124-
Also use 👍 to highlight particularly good code or design decisions.
120+
| Emoji | Tag | Use when |
121+
| ----- | ------------ | ------------------------------------------------------------------------------ |
122+
| 🔧 | **wrench** | A necessary change: bugs, data loss, security, missing validation, CI failures |
123+
|| **question** | A question that must be answered before you can complete the review |
124+
125+
#### Non-blocking (merge can proceed)
126+
127+
| Emoji | Tag | Use when |
128+
| ----- | ---------------- | -------------------------------------------------------------------------- |
129+
| 🤔 | **thinking** | Thinking aloud — expressing a concern or exploring alternatives |
130+
| ♻️ | **refactor** | A concrete refactoring suggestion with enough context to act on |
131+
| 🌱 | **seedling** | A future-focused observation — not for this PR but worth considering |
132+
| 📝 | **note** | An explanatory comment or context — no action required |
133+
|| **nitpick** | A stylistic or formatting preference — does not require changes |
134+
| 🏕 | **camp site** | An opportunity to leave the code better than you found it (boy scout rule) |
135+
| 📌 | **out of scope** | An important concern outside this PR's scope — needs a follow-up issue |
136+
| 👍 | **praise** | Highlight particularly good code, design, or testing decisions |
125137

126138
### 6. Present findings for user approval
127139

128140
**Do not submit the review automatically.** Present all findings to the user
129141
organized by severity, with:
130142

131-
- Severity and title
143+
- Emoji tag and title
132144
- File path and line number
133145
- Description and suggested fix
134146
- Whether it would be an inline comment or body-level finding
135147

148+
Group findings into two sections: **Blocking** (🔧 / ❓) and **Non-blocking**
149+
(everything else). This makes it immediately clear what must be addressed.
150+
136151
Ask the user which findings to include in the PR review. The user may:
137152

138153
- Approve all findings
139154
- Exclude specific findings
140-
- Adjust severity levels
155+
- Change emoji tags
141156
- Edit descriptions
142157
- Add additional comments
143158

@@ -149,10 +164,10 @@ After user approval, submit the selected findings as a formal review.
149164

150165
#### Determine the review verdict
151166

152-
- If any P0 findings are included: `CHANGES_REQUESTED`
153-
- If any P1 findings are included: `CHANGES_REQUESTED`
154-
- If only P2 or below: `COMMENT`
155-
- If no findings: `APPROVE`
167+
- If any 🔧 (wrench) findings are included: `REQUEST_CHANGES`
168+
- If any ❓ (question) findings are included: `COMMENT` (questions need answers, not change requests)
169+
- If only non-blocking findings (🤔 ♻️ 🌱 📝 ⛏ 🏕 📌 👍): `COMMENT`
170+
- If no findings (or only 👍 praise): `APPROVE`
156171

157172
#### Build inline comments
158173

@@ -165,7 +180,7 @@ comment. Use the file's **current line number** (not diff position) with the
165180
"path": "crates/common/src/publisher.rs",
166181
"line": 166,
167182
"side": "RIGHT",
168-
"body": "🔧 **Race condition**: Description of the issue...\n\n**Fix**:\n```rust\n// suggested code\n```"
183+
"body": "🔧 **wrench** — Race condition: Description of the issue...\n\n**Fix**:\n```rust\n// suggested code\n```"
169184
}
170185
````
171186

@@ -179,24 +194,38 @@ concerns, architectural issues, dependency problems) in the review body:
179194

180195
<1-2 sentence overview of the changes and overall assessment>
181196

182-
## Findings
197+
## Blocking
198+
199+
### 🔧 wrench
200+
201+
- **Title**: description (file:line)
202+
203+
### ❓ question
183204

184-
### 🔧 Blockers (P0)
205+
- **Title**: description (file:line)
206+
207+
## Non-blocking
208+
209+
### 🤔 thinking
185210

186211
- **Title**: description (file:line)
187212

188-
### 🔧 High (P1)
213+
### ♻️ refactor
189214

190215
- **Title**: description (file:line)
191216

192-
### 🤔 Medium (P2)
217+
### 🌱 seedling / 🏕 camp site / 📌 out of scope
193218

194219
- **Title**: description
195220

196-
### Low (P3)
221+
### nitpick
197222

198223
- **Title**: description
199224

225+
### 👍 praise
226+
227+
- **Title**: description (file:line)
228+
200229
## CI Status
201230

202231
- fmt: PASS/FAIL
@@ -205,6 +234,8 @@ concerns, architectural issues, dependency problems) in the review body:
205234
- js tests: PASS/FAIL
206235
```
207236

237+
Omit any section that has no findings — don't include empty headings.
238+
208239
#### Submit the review
209240

210241
Use the GitHub API to submit. Handle these known issues:
@@ -242,8 +273,8 @@ Where `comments.json` contains the array of inline comment objects.
242273
Output:
243274

244275
- The review URL
245-
- Total findings by severity (e.g., "2 P0, 3 P1, 5 P2, 2 P3")
246-
- Whether the review requested changes or approved
276+
- Total findings by category (e.g., "2 🔧, 1 ❓, 3 🤔, 2 ⛏, 1 👍")
277+
- Whether the review requested changes, commented, or approved
247278
- Any CI failures encountered
248279

249280
## Rules

.github/ISSUE_TEMPLATE/bug_report.yml

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -15,61 +15,3 @@ body:
1515
placeholder: What happened?
1616
validations:
1717
required: true
18-
19-
- type: textarea
20-
id: reproduction
21-
attributes:
22-
label: Steps to reproduce
23-
description: Minimal steps to trigger the bug.
24-
placeholder: |
25-
1. Configure trusted-server.toml with ...
26-
2. Run `fastly compute serve`
27-
3. Send a request to ...
28-
4. See error ...
29-
validations:
30-
required: true
31-
32-
- type: textarea
33-
id: expected
34-
attributes:
35-
label: Expected behavior
36-
description: What should have happened instead?
37-
validations:
38-
required: true
39-
40-
- type: dropdown
41-
id: area
42-
attributes:
43-
label: Affected area
44-
description: Which part of the project is affected?
45-
multiple: true
46-
options:
47-
- Core (synthetic IDs, cookies, GDPR)
48-
- Integrations (prebid, lockr, permutive, etc.)
49-
- HTML processing / JS injection
50-
- Ad serving (Equativ)
51-
- Fastly runtime
52-
- JS build pipeline
53-
- CI / Tooling
54-
validations:
55-
required: true
56-
57-
- type: input
58-
id: version
59-
attributes:
60-
label: Version
61-
description: Git SHA or version.
62-
placeholder: "commit abc1234"
63-
64-
- type: textarea
65-
id: logs
66-
attributes:
67-
label: Relevant log output
68-
description: Paste any error messages or logs.
69-
render: shell
70-
71-
- type: textarea
72-
id: context
73-
attributes:
74-
label: Additional context
75-
description: Anything else that might help (OS, Rust version, related issues).

.github/ISSUE_TEMPLATE/story.yml

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,44 +15,3 @@ body:
1515
placeholder: "As a publisher, I want ... so that ..."
1616
validations:
1717
required: true
18-
19-
- type: textarea
20-
id: acceptance-criteria
21-
attributes:
22-
label: Acceptance criteria
23-
description: What must be true for this story to be considered done?
24-
placeholder: |
25-
- [ ] Criterion 1
26-
- [ ] Criterion 2
27-
validations:
28-
required: true
29-
30-
- type: dropdown
31-
id: scope
32-
attributes:
33-
label: Affected area
34-
description: Which part of the project would this touch?
35-
multiple: true
36-
options:
37-
- Core (synthetic IDs, cookies, GDPR)
38-
- Integrations (prebid, lockr, permutive, etc.)
39-
- HTML processing / JS injection
40-
- Ad serving (Equativ)
41-
- Fastly runtime
42-
- JS build pipeline
43-
- Documentation
44-
- CI / Tooling
45-
validations:
46-
required: true
47-
48-
- type: textarea
49-
id: solution
50-
attributes:
51-
label: Proposed approach
52-
description: How should this be implemented? Include API examples if relevant.
53-
54-
- type: textarea
55-
id: context
56-
attributes:
57-
label: Additional context
58-
description: Links, mockups, related issues, or prior art.

.github/ISSUE_TEMPLATE/task.yml

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,38 +14,3 @@ body:
1414
description: What needs to be done and why?
1515
validations:
1616
required: true
17-
18-
- type: textarea
19-
id: done-criteria
20-
attributes:
21-
label: Done when
22-
description: How do we know this task is complete?
23-
placeholder: |
24-
- [ ] Criterion 1
25-
- [ ] Criterion 2
26-
validations:
27-
required: true
28-
29-
- type: dropdown
30-
id: scope
31-
attributes:
32-
label: Affected area
33-
description: Which part of the project would this touch?
34-
multiple: true
35-
options:
36-
- Core (synthetic IDs, cookies, GDPR)
37-
- Integrations (prebid, lockr, permutive, etc.)
38-
- HTML processing / JS injection
39-
- Ad serving (Equativ)
40-
- Fastly runtime
41-
- JS build pipeline
42-
- Documentation
43-
- CI / Tooling
44-
validations:
45-
required: true
46-
47-
- type: textarea
48-
id: context
49-
attributes:
50-
label: Additional context
51-
description: Related issues, dependencies, or notes.

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)