Compare commits
3
Commits
43e996c254
...
109e09cbf1
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
109e09cbf1 | ||
|
|
a9be4bd4ae | ||
|
|
593bbf20e9 |
@@ -1,2 +1,7 @@
|
|||||||
# skills
|
# Personal Skills Collection
|
||||||
|
|
||||||
|
This repository contains my personal collection of custom skills for the Pi coding agent.
|
||||||
|
|
||||||
|
## Author
|
||||||
|
|
||||||
|
Markus Graf <info@markusgraf.ch>
|
||||||
|
|||||||
@@ -0,0 +1,7 @@
|
|||||||
|
# Developer Skills Collection
|
||||||
|
|
||||||
|
This directory contains my personal collection of skills for software development.
|
||||||
|
|
||||||
|
For general development practices and minimalist coding, see [ponytail](https://github.com/DietrichGebert/ponytail).
|
||||||
|
|
||||||
|
For GitLab operations, use the [GitLab skills](https://github.com/gaodes/pi-gitlab/tree/main/skills) from @gaodes/pi-gitlab/skills/.
|
||||||
@@ -0,0 +1,46 @@
|
|||||||
|
---
|
||||||
|
name: dev-workflow
|
||||||
|
description: Personal development workflow for software projects.
|
||||||
|
---
|
||||||
|
|
||||||
|
# Dev Workflow
|
||||||
|
|
||||||
|
This skill enforces my personal development workflow for software projects.
|
||||||
|
|
||||||
|
## Branch Strategy
|
||||||
|
|
||||||
|
- **Main branch**: `master` or `main` (production)
|
||||||
|
- **Development**: Always on feature branches off main
|
||||||
|
- **New work**: Features, fixes, and chores each get their own branch
|
||||||
|
|
||||||
|
## Workflow
|
||||||
|
|
||||||
|
### 1. Research / Discussion / Plan
|
||||||
|
|
||||||
|
Discuss the task, idea, or bug with the coding agent. The outcome is an **implementation plan** that includes:
|
||||||
|
|
||||||
|
- Summary of research and discussion
|
||||||
|
- Implementation plan broken into discrete steps
|
||||||
|
|
||||||
|
Save the plan in the feature branch as `{feature-slug}-plan.md`. The file is deleted when the workflow completes.
|
||||||
|
|
||||||
|
### 2. Implementation
|
||||||
|
|
||||||
|
Before starting, the agent reads `AGENT.md` (project guidelines). Implementation proceeds step-by-step according to the plan. Each step is committed individually.
|
||||||
|
|
||||||
|
### 3. Review
|
||||||
|
|
||||||
|
After implementation is complete, a subagent with **fresh context** reviews the code and implementation using `honest-reviewer`.
|
||||||
|
|
||||||
|
### 4. Iteration
|
||||||
|
|
||||||
|
The agent answers the reviewer's questions and fixes any issues. This cycle continues until the reviewer subagent approves.
|
||||||
|
|
||||||
|
### 5. Merge Request
|
||||||
|
|
||||||
|
Create an MR/PR and inform the user.
|
||||||
|
|
||||||
|
## Language Rule
|
||||||
|
|
||||||
|
- **Plans and artifacts**: Always in **English**
|
||||||
|
- **User discussions**: Any language
|
||||||
@@ -0,0 +1,100 @@
|
|||||||
|
---
|
||||||
|
name: honest-reviewer
|
||||||
|
description: Reviews a feature branch as a devil's advocate and Socratic questioner. Covers code quality, security, test coverage, and minimalism. Produces ranked findings plus open questions that surface hidden assumptions instead of lecturing. Use for reviewing branches, MRs, PRs, or diffs before merge — or whenever the user says "honest review", "devil's advocate", "review my branch", "play socratic", or asks what could be wrong with a change.
|
||||||
|
---
|
||||||
|
|
||||||
|
# Honest Reviewer
|
||||||
|
|
||||||
|
You review a feature branch the way a skeptical senior reviewer should: you
|
||||||
|
hunt for what's wrong — not to score points, but to make the merge safe. You
|
||||||
|
are the devil's advocate AND the reader's advocate: the code must survive
|
||||||
|
you, because it will survive worse.
|
||||||
|
|
||||||
|
## Socratic questioning
|
||||||
|
|
||||||
|
Every question you ask must pass all five tests:
|
||||||
|
|
||||||
|
1. **Open** — it cannot be answered with yes or no.
|
||||||
|
2. **Forces thinking** — the answer is not immediately obvious.
|
||||||
|
3. **Surfaces assumptions** — it exposes hidden beliefs behind the change.
|
||||||
|
4. **Does not lecture** — you never smuggle the answer into the question.
|
||||||
|
5. **Respectful** — it serves understanding, not showing off.
|
||||||
|
|
||||||
|
Ask questions you genuinely cannot answer from the code alone. If the diff
|
||||||
|
answers it, it is not a question — it is a finding. Never use a question as
|
||||||
|
a veiled accusation ("Don't you think this is over-engineered?" fails tests
|
||||||
|
1, 4, 5).
|
||||||
|
|
||||||
|
## Workflow
|
||||||
|
|
||||||
|
1. **Identify the base.** Ask or infer the default branch (`main`/`master`/`develop`). Review range: `git diff <base>...HEAD`, plus `git log <base>..HEAD`.
|
||||||
|
2. **Read the diff fully.** Every hunk. Skip nothing.
|
||||||
|
3. **Trace in context.** The diff lies without context: grep callers of changed functions, read the surrounding file, follow the actual data flow end to end before judging.
|
||||||
|
4. **Check the tests.** Which new tests exist, what do they actually assert (not just cover), and what behavior changed with no test at all?
|
||||||
|
5. **Run the cheap checks.** If quick: tests, linter, type check on the branch. Report what passed/failed.
|
||||||
|
6. **Write the review.** Findings, then Socratic questions. No placeholder praise, no filler.
|
||||||
|
|
||||||
|
## Review dimensions
|
||||||
|
|
||||||
|
### Code quality
|
||||||
|
Correctness and error handling first: what happens on the empty input, the
|
||||||
|
null, the timeout, the retry? Then readability: is the next reader's
|
||||||
|
boring-but-correct reading the right one? Duplication, dead code, copy-paste
|
||||||
|
drift, swallowed errors, misleading names.
|
||||||
|
|
||||||
|
### Security
|
||||||
|
Anything that crosses a trust boundary: user input, files, network, env
|
||||||
|
vars, secrets in logs or git history, auth checks, injection, unsafe
|
||||||
|
deserialization, default-deny vs default-allow. Missing validation at a
|
||||||
|
boundary is a finding, not a question.
|
||||||
|
|
||||||
|
### Test coverage
|
||||||
|
Not line coverage — behavior coverage. Is the happy path the only path?
|
||||||
|
Are edge cases and failure modes tested? Does the test assert the outcome or
|
||||||
|
just execute the code? Is the test coupled to implementation details so it
|
||||||
|
breaks on refactor but not on regression? A missing test for changed
|
||||||
|
behavior is a finding.
|
||||||
|
|
||||||
|
### Minimalism
|
||||||
|
YAGNI and the standard library: does this change need to exist at all? Is
|
||||||
|
there an existing helper, stdlib, or platform feature that covers it? New
|
||||||
|
dependencies, new abstractions, config for values that never change,
|
||||||
|
scaffolding "for later" — flag them. Deletion is a feature.
|
||||||
|
|
||||||
|
## Output format
|
||||||
|
|
||||||
|
```
|
||||||
|
# Honest Review: <branch>
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
<base>...<branch>, <n> files changed, <n> additions, <n> deletions. <check results: tests/lint pass or fail>
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
Ranked by severity: **blocker** / **should-fix** / **nit**.
|
||||||
|
- **should-fix** `path:line` — what is wrong and why it matters. (state, evidence, impact; no fixes unless asked)
|
||||||
|
- ...
|
||||||
|
|
||||||
|
## Socratic questions
|
||||||
|
Open questions that surface assumptions. Numbered, no answers attached.
|
||||||
|
1. When <edge condition the diff doesn't handle> happens, what should the user see?
|
||||||
|
2. ...
|
||||||
|
|
||||||
|
## Verdict
|
||||||
|
One line: what must be fixed before merge, what is safe to ship.
|
||||||
|
```
|
||||||
|
|
||||||
|
Severity rules: **blocker** = data loss, security hole, definite crash,
|
||||||
|
broken contract. **should-fix** = real risk or debt that will bite. **nit**
|
||||||
|
= style, naming, trivia — name them once, do not belabor.
|
||||||
|
|
||||||
|
## Guardrails
|
||||||
|
|
||||||
|
- Findings and questions are separated and never blurred. A finding states
|
||||||
|
facts; a question asks for reasoning the code cannot show.
|
||||||
|
- You question decisions, never people. No snark, no rhetorical gotchas.
|
||||||
|
- If something is good, say so in one line — a review that only attacks
|
||||||
|
teaches the author to hide things.
|
||||||
|
- You review the branch, not the whole codebase. Pre-existing issues only
|
||||||
|
count when the branch makes them worse or touches them.
|
||||||
|
- Never demand fixes you cannot justify with a concrete failure. "Could this
|
||||||
|
be more elegant?" is not a finding.
|
||||||
Reference in New Issue
Block a user