You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
chore: trim generic-bloat and genericize NR-internal examples (#29)
Part of netresearch/skill-repo-skill#157; ticks the peer-qa-review
checkbox.
## Rubric
Judged against skill-repo-skill's [content value
rubric](https://github.com/netresearch/skill-repo-skill/blob/main/skills/skill-repo/references/skill-quality.md#content-value-rubric)
(six value categories; "generic bloat" = restated public best practice a
one-line prompt regenerates).
## Cut
- **`references/frameworks.md`** (532 words) — deleted. It mapped this
skill's pillars against ITIL PIR, Scrum DoD, Google SRE postmortems, and
NIST 800-128. None of the rubric's six value categories apply: no
org/project specifics, no retro-born failure pattern, no executable
content, no inference-suppression rule, no version/ecosystem fact. The
file's own framing ("naming the alignment helps with credibility and
onboarding") confirms it's credibility dressing, not guidance a reviewer
acts on. Removed the SKILL.md reference-table entry; no other file
linked to it (`audit-skills.sh` confirms 0 orphans post-removal).
## Genericized (not cut — this is a public generic skill, not
org-specific bloat)
The skill's worked examples carried real Netresearch internals, which
don't belong in a public "generic for any IT/Ops team" skill:
- Internal Jira project keys (`NRS-*`, `NRT-*`, `SRVV-*`, `IOT-*`) →
placeholder tickets (`PROJ-*`, `OPS-*`, `INV-*`, `INFRA-*`)
- Real reviewer names in citations (Björn Marten, Sebastian Mendel) →
dropped
- Internal hostname (`exocortex.nr`) → `vault01.internal`
- Internal GitLab MR URL (`git.netresearch.de/provision/...`) →
`git.example.com/infra/...`
- Internal GHCR/GitHub org (`ghcr.io/netresearch`,
`github.com/netresearch/<component>`) → `example-org`
- Internal Jira custom-field name (`IOS`) → generic "a Current Version
custom field"
- "many NR teams" → "many teams"
- The QA2 customer-handover worked example carried a real customer name,
a real Jira host, and two real employee names → German placeholder names
(Max Mustermann / Erika Musterfrau, standard German "John Doe"
equivalents), `jira.example.com`, generic customer/approver
placeholders. Kept the German-language text itself — it's the
pedagogical point ("matches the requester's language").
Left `scripts/qa-gather.sh` untouched: its
`netresearch-claude-code-marketplace` search path and
`github.com/netresearch/jira-skill` pointer are real, live, functional
values (verified: the marketplace directory exists locally, the
jira-skill repo returns HTTP 200), not internal-only leakage, and the
extra search-root is load-bearing for `find`'s maxdepth reachability,
not a stylistic default.
## Kept, borderline
- **`scripts/qa-gather.sh`**'s NR-specific search paths (see above) —
functional, not prose leakage; removing risks a real regression.
- **CHANGELOG.md**'s historical NR ticket/dogfooding references
(`NRS-4365` dogfood entries etc.) — left untouched as a historical
record of real development; out of scope (task targets skill guidance
content, not changelog history), and rewriting past changelog entries
isn't warranted.
- **`references/checklist.md`, `references/edge-cases.md`,
`references/comment-template.md`, `references/anti-patterns.md`** bodies
— kept in full. These hold retro-born failure patterns (e.g. F4a's
"seeing a ticket key in prose ≠ linked" self-deception, the
display-text-link anti-pattern, the
QA2-handover-is-not-the-internal-comment rule) and inference-suppression
rules ("verify against the actual issue-link list, not the
description"), both hard-protected categories.
- **`references/anti-patterns.md` item 12** (verifier-FAIL-dismissal
pattern, added upstream in #28 after this branch was cut) — a genuine
retro-born failure pattern with a concrete incident (an offboarding
batch where a coarse `AD` FAIL was nearly dismissed as a false
positive). Untouched, no NR-specific leak in it.
## Verification
Re-verified every file/line fact against current `origin/main`
(`0b76905`) before cutting — the local worktree was stale on an older
release branch; re-fetched and re-read the current tree.
```
$ bash validate-skill.sh . # from origin/main skill-repo-skill (local copy was stale)
...
Errors: 0
Warnings: 10 # pre-existing (README template sections, checkpoints.yaml) — unrelated to this change
$ bash audit-skills.sh .
DESCRIPTION: 795 chars [WARN] # pre-existing, untouched by this PR
BODY: 394 words, 69 lines [PASS]
GENERIC SHARE: 0% (0/11 paragraphs generic, 1 protected) [PASS]
REFERENCES: 6 total -> 6 reachable, 0 ORPHAN
$ wc -w skills/peer-qa-review/SKILL.md
497 # was 498; under the 500-word cap either way
$ pre-commit run --files <all changed .md>
markdownlint-cli2 Passed
Validate skill repo structure Passed
Plugin/SKILL/composer version parity Passed
$ shellcheck skills/peer-qa-review/scripts/qa-gather.sh && bash -n <same>
(clean — script untouched)
```
Word counts (before → after):
| File | Before | After |
|---|---|---|
| `SKILL.md` | 498 | 497 |
| `references/frameworks.md` | 532 | deleted |
| `references/severity.md` | 1152 | 1143 |
| `references/anti-patterns.md` | 674 | 673 |
| `references/comment-template.md` | 2634 | 2634 (substitution, not cut)
|
| `references/edge-cases.md` | 1220 | 1220 (substitution, not cut) |
| `references/checklist.md` | 2173 | 2173 (substitution, not cut) |
Grepped the whole repo for the cut/genericized identifiers (`NRS-`,
`NRT-`, `SRVV-`, `IOT-`, `exocortex`, `Björn`, `Sebastian Mendel`,
`Tobias`, `Fischer`, `Strobel`, `BBAG`, `jira.netresearch`,
`git.netresearch.de/<path>`) across `skills/` — 0 remaining hits outside
`CHANGELOG.md` (left as historical record, out of scope).
Copy file name to clipboardExpand all lines: skills/peer-qa-review/references/anti-patterns.md
+1-1Lines changed: 1 addition & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -7,7 +7,7 @@ Things to flag in the implementer's comments. Each item below is `(!)` unless no
7
7
1.**One giant final comment** instead of one-per-step. Kills the audit trail when work is interrupted; reviewers can't tell what was done when.
8
8
2.**`{{monospace}}`** for commands instead of `{code}` blocks. Can't be expanded/collapsed, no syntax highlighting, copy-paste eats whitespace.
9
9
3.**Markdown leakage** in Jira: `**bold**`, `# heading`, em-dashes `--`, `[display text](url)`. Renders as literal text.
10
-
4.**`[display text|url]`** display-text links. Convention in many NR teams is full URLs — the URL shows where it goes, the text becomes a lie when the link is renamed.
10
+
4.**`[display text|url]`** display-text links. Convention in many teams is full URLs — the URL shows where it goes, the text becomes a lie when the link is renamed.
11
11
5.**No prompt context in `{code}` blocks**: just bare output. The reader can't tell which host or container the command ran in.
12
12
6.**Mixing implementer and reviewer voice in one thread** without clear `h3.` headings to separate concerns.
Copy file name to clipboardExpand all lines: skills/peer-qa-review/references/checklist.md
+2-2Lines changed: 2 additions & 2 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -19,7 +19,7 @@ All checks, organised by pillar. Apply the severity in the rightmost column when
19
19
20
20
Note F1: "acceptance criteria absent" is the one finding you should bounce on without going further. Without criteria, there is no testable bar.
21
21
22
-
Note F4a: don't conflate "mentioned in description prose" with "linked via issue-link feature". A common self-deception is reading the description, seeing `IOT-146`, and assuming it must be linked. **Always verify against the actual issue-link list** (Jira: `Issue Links` section / API `issuelinks` array). If a related ticket is mentioned in prose but not linked, that's a `(!)` — and an easy fix (add the link).
22
+
Note F4a: don't conflate "mentioned in description prose" with "linked via issue-link feature". A common self-deception is reading the description, seeing `INV-146`, and assuming it must be linked. **Always verify against the actual issue-link list** (Jira: `Issue Links` section / API `issuelinks` array). If a related ticket is mentioned in prose but not linked, that's a `(!)` — and an easy fix (add the link).
23
23
24
24
Note F4a (reviewer-side): apply the same rule to *your own* QA comment. Every ticket key, MR/PR URL, commit hash, runbook page, or vault entry you mention as a reviewer should also exist as a structural link on the ticket. If the QA comment introduces a *new* reference — typical case: "filed follow-up as NEW-TICKET", "see MR !N", "fixed by commit abc123" — create the structural link (issue link for tickets, web link for external URLs) *before* posting the comment, not after. The inline mention is for narrative; the link is the relationship that survives someone navigating in from the *other* side. Anti-pattern caught the hard way: QA comments referencing a follow-up ticket only inline, leaving the navigation one-way.
25
25
@@ -75,7 +75,7 @@ For changes with no plausible adjacent surface (e.g. a typo fix in a comment, a
75
75
76
76
| # | Check | Severity |
77
77
|---|-------|----------|
78
-
| I1 |**Inventory / CMDB** entry updated where applicable (e.g. IOS "Current Version" custom field for maintenance tickets) |`(x)` for maintenance tickets if missing; `(!)` otherwise |
78
+
| I1 |**Inventory / CMDB** entry updated where applicable (e.g. a "Current Version" custom field for maintenance tickets) |`(x)` for maintenance tickets if missing; `(!)` otherwise |
79
79
| I2 |**Linked tickets state**: parent epic moves forward, child tickets are themselves in valid states, inventory issues reflect the change |`(!)` per inconsistency |
80
80
| I3 |**Sibling tickets**: if this is one of N parallel tickets (e.g. multiple host upgrades), spot-check that this ticket's pattern matches the others |`(i)` — flag deviation |
81
81
| I4 |**Side-quests** (improvements made en passant) are documented as separate sub-headings, not lost |`(i)`|
Copy file name to clipboardExpand all lines: skills/peer-qa-review/references/comment-template.md
+21-21Lines changed: 21 additions & 21 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -4,7 +4,7 @@ One structured comment at the end of QA. Use Jira wiki markup. Optional addendum
4
4
5
5
## Link conventions
6
6
7
-
For your team's *trusted shared-namespace* GitHub / GitLab projects, prefer the platform's native shorthand over full URLs. Same logic as why we already use bare `NRS-4365` instead of full Jira URLs: shorter, more scannable, project-context inline.
7
+
For your team's *trusted shared-namespace* GitHub / GitLab projects, prefer the platform's native shorthand over full URLs. Same logic as why teams often use bare `PROJ-4365` instead of full Jira URLs: shorter, more scannable, project-context inline.
@@ -20,13 +20,13 @@ For your team's *trusted shared-namespace* GitHub / GitLab projects, prefer the
20
20
**In Jira specifically**: the shorthand alone is *not* clickable — Jira only auto-links its own issue keys. To get clickable links *and* shorthand readability in Jira, wrap the shorthand as **display text** in a Jira link macro:
This renders as a clickable link reading `provision/ansible-role-vault!9` — the same anchor text GitHub/GitLab use natively. **This is *not* the display-text-link anti-pattern** (which targets opaque text like `[click here|url]`) — the shorthand IS the canonical reference, so using it as display text is the *opposite* of opaque.
27
+
This renders as a clickable link reading `infra/deploy-tooling!9` — the same anchor text GitHub/GitLab use natively. **This is *not* the display-text-link anti-pattern** (which targets opaque text like `[click here|url]`) — the shorthand IS the canonical reference, so using it as display text is the *opposite* of opaque.
28
28
29
-
**On GitHub PRs / GitLab MRs**: the bare shorthand is already auto-linked by the platform, so write `provision/ansible-role-vault!9` (without the `[…|…]` wrapper) when authoring there.
29
+
**On GitHub PRs / GitLab MRs**: the bare shorthand is already auto-linked by the platform, so write `infra/deploy-tooling!9` (without the `[…|…]` wrapper) when authoring there.
30
30
31
31
**Rule of thumb**:
32
32
-*Authoring in Jira* → wrap: `[shorthand|url]`
@@ -51,9 +51,9 @@ Use `*` (or `**` for sub-items) at the start of each finding line — *not* bare
51
51
h4. Formal correctness
52
52
* (/) F1: Description has clear acceptance criteria
53
53
* (/) F4a: Structured Jira issue links:
54
-
** NRS-4317 — parent, Closed
55
-
** IOT-146 — VM inventory, In use
56
-
** SRVV-104 — related, Closed
54
+
** PROJ-4317 — parent, Closed
55
+
** INV-146 — VM inventory, In use
56
+
** INFRA-104 — related, Closed
57
57
* (!) F6: No worklog entries — should-have per audit/billing/capacity
58
58
```
59
59
@@ -148,35 +148,35 @@ Re-read your own comment before clicking *Add*. Common self-introduced bugs:
148
148
9.**Self-fixed findings carry paired icons** — a finding you fixed yourself during QA is written as `(!) finding — (/) fixed <how> during QA` (or with the fix as a nested `**` sub-item carrying its evidence link). Neither a bare `(/)` nor a bare `(!)` tells the whole story; see `severity.md` ("Findings fixed by the reviewer"). Every MR, commit and repo you name in the finding gets its `[shorthand|url]` link.
149
149
10.**QA2 verdict but no customer handover** — if the verdict routes to QA2, the internal QA comment is *not* enough; a separate plain-language handover for the approver must accompany it (see § "Customer handover comment (QA2 only)"). Posting only the internal QA comment leaves the customer lost.
150
150
151
-
## Example 1 — Pass (NRS-4365 shape)
151
+
## Example 1 — Pass (PROJ-4365 shape)
152
152
153
153
```jira
154
154
h3. IT Internal QA — passed
155
155
156
156
h4. Formal correctness
157
157
(/) Description has clear acceptance criteria (4 numbered tasks)
158
158
(/) Implementer comments document each step with command+output
GitHub releases v1.2.1 and v1.2.2 are tagged but no Docker images were published. Only 1.2.0 and latest are on ghcr.io.
244
244
245
245
h4. Reopen condition
246
-
The CI for https://github.com/netresearch/<component> doesn't publish Docker images on release. Reopen this ticket once the GitHub Actions release workflow is fixed and v1.2.2 (or later) images are available.
246
+
The CI for https://github.com/example-org/<component> doesn't publish Docker images on release. Reopen this ticket once the GitHub Actions release workflow is fixed and v1.2.2 (or later) images are available.
247
247
248
248
h4. Follow-up filed
249
249
{NEW-TICKET}: fix GitHub Actions release workflow for <component> to publish Docker images.
@@ -321,14 +321,14 @@ h3. Handover for [~approver.username]
321
321
Sobald das passt, {next step: "dürfen Sie das Ticket schließen" | "geben Sie uns kurz Bescheid und wir schließen ab"}. Bei Rückfragen: {contact / channel}.
322
322
```
323
323
324
-
### Example — QA2 handover (NRS-4480 shape)
324
+
### Example — QA2 handover (PROJ-4480 shape)
325
325
326
326
```jira
327
-
h3. Handover for [~tobias.hein]
327
+
h3. Handover for [~approver.username]
328
328
329
-
Der Jira-Zugang für Reinhold Fischer (BBAG) ist eingerichtet — Benutzer aktiv, Rechte analog zu Josef Strobel, Zugangsdaten wurden Herrn Fischer zugestellt.
329
+
Der Jira-Zugang für Max Mustermann (Kunde GmbH) ist eingerichtet — Benutzer aktiv, Rechte analog zu Erika Musterfrau, Zugangsdaten wurden Herrn Mustermann zugestellt.
330
330
331
-
*Bitte zur Abnahme prüfen:* dass Herr Fischer sich unter https://jira.netresearch.de mit den zugestellten Zugangsdaten einloggen kann.
331
+
*Bitte zur Abnahme prüfen:* dass Herr Mustermann sich unter https://jira.example.com mit den zugestellten Zugangsdaten einloggen kann.
332
332
333
333
Sobald die Anmeldung bestätigt ist, kann das Ticket geschlossen werden. Bei Rückfragen meldet euch gern.
Copy file name to clipboardExpand all lines: skills/peer-qa-review/references/severity.md
+6-6Lines changed: 6 additions & 6 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -60,29 +60,29 @@ Developers are humans too — when QA turns up a genuinely *positive unexpected*
60
60
61
61
Independently, the **closing sentence** of the QA comment may carry a 1–5 star rating of the overall implementation quality, e.g. `(*)(*)(*)(*) (4/5)`. Rough scale: 5 = exemplary, fit as a reference for others; 4 = strong with a minor flaw; 3 = solid standard work; 2 = passed with notable gaps; 1 = barely passed. The rating is optional and never replaces the verdict line.
62
62
63
-
## Examples from real tickets
63
+
## Examples
64
64
65
65
### `(x)` — blocking
66
66
67
67
> *(x) container image for 1.2.2 does not exist on ghcr.io. Cannot deploy without a published image.*
68
-
> — NRT-4567, resolved as Won't-do
68
+
> — OPS-4567, resolved as Won't-do
69
69
70
70
> *(x) F1: Description has no acceptance criteria. Cannot QA — bouncing to In Progress for clarification.*
71
71
72
72
### `(!)` — should-fix
73
73
74
74
> *(!) molecule.yml references requirements.yml but file doesn't exist. Generates warning. Recommend either creating an empty requirements.yml or removing the dependency block.*
75
-
> — NRS-4199 QA review (Björn Marten)
75
+
> — PROJ-4199 QA review
76
76
77
77
> *(!) :latest image tags in CI — components are pinned (@v0.2.0) but Docker images are :latest. A breaking rebuild upstream would silently propagate.*
78
-
> — NRS-4356 QA review (Sebastian Mendel)
78
+
> — PROJ-4356 QA review
79
79
80
80
### `(i)` — hint
81
81
82
82
> *(i) timezone test missing `when: setup_time` guard. Other feature tests use this pattern — should be consistent.*
83
83
84
-
> *(i) same cgroup v1 legacy warning as IOT-71 — not blocking, worker runs fine after 30s v2 fallback.*
85
-
> — NRS-4240 QA review (Björn Marten)
84
+
> *(i) same cgroup v1 legacy warning as INV-71 — not blocking, worker runs fine after 30s v2 fallback.*
0 commit comments