code-review-line — Построчное ревью
Формат подобран так, чтобы находку можно было прочитать за секунду и сразу понять, что делать.
Формат
Одна находка — одна строка:
internal/auth/token.go:91: 🔴 bug: exp сравнивается с локальным временем, не UTC. Взять time.Now().UTC().
путь:строка: <эмодзи> <уровень>: <проблема>. <действие>.
Уровни:
|
уровень |
значение |
| 🔴 |
bug |
сломается или уже сломано |
| 🟡 |
risk |
работает сейчас, развалится при изменении условий |
| 🔵 |
nit |
стиль, именование, читаемость |
| ❓ |
q |
непонятно намерение, нужен ответ автора |
Последней строкой итог: totals: 1 bug, 2 risk, 3 nit, 1 q.
Нет находок — ответ No issues. целиком. Пустое ревью лучше выдуманного.
Что смотреть и в каком порядке
Порядок отражает цену ошибки:
- Корректность — делает ли код то, что заявлено
- Границы — пустые данные, нули, переполнения, отсутствующие ключи
- Ошибки — что происходит на неуспешной ветке, не проглатывается ли исключение
- Конкурентность — гонки, дедлоки, общее состояние
- Безопасность — данные из недоверенного источника, секреты, права
- Читаемость — имена, длина функций, комментарии
Отдельно ищи то, чего в diff не видно: сломанные инварианты, места, где новый код противоречит соседнему, обработчики, которые надо было обновить вместе с этим.
Где формат разворачивается
Правило ../../rules/agentops-auto-clarity.md применимо и здесь. Однострочный формат выключается, когда находка касается:
- безопасности — уязвимость объясняется целиком, с вектором и последствиями
- архитектурного спора — если проблема в подходе, а не в строке, одной строкой её не сформулировать
- онбординга — когда автор новичок и ему нужна не пометка, а объяснение
В этих случаях после однострочной находки идёт развёрнутый абзац. Формат — инструмент экономии, а не самоцель.
Чего не делать
- Не хвали. «Хорошая декомпозиция» не меняет ни строки кода.
- Не пересказывай изменения. Автор их написал.
- Не помечай
bug то, в чём не уверен: есть risk и q.
- Не предлагай переписать всё иначе — для этого отдельный разговор, а не комментарий к строке.
- Не собирай находки ради непустого списка.
- Не ставь
nit там, где в проекте нет соответствующего соглашения: это твой вкус, а не правило.
1---2name: code-review-line3description: Построчные комментарии к diff или PR: одна находка — одна строка, с уровнем серьёзности и конкретным действием. Никакой похвалы и пересказа изменений. Используй, когда пользователь говорит «отревьюь diff», «посмотри изменения», «что не так в этом PR», «код-ревью», «проверь перед коммитом». Для ревью плана до реализации — code-senior-review, здесь только существующий код.4---56<!-- СГЕНЕРИРОВАНО bin/mirror.js. Не редактировать: правки затрёт следующая генерация.7 Источник правды — domains/<домен>/. -->89# code-review-line — Построчное ревью1011Формат подобран так, чтобы находку можно было прочитать за секунду и сразу понять, что делать.1213## Формат1415Одна находка — одна строка:1617```18internal/auth/token.go:91: 🔴 bug: exp сравнивается с локальным временем, не UTC. Взять time.Now().UTC().19```2021`путь:строка: <эмодзи> <уровень>: <проблема>. <действие>.`2223Уровни:2425| | уровень | значение |26|---|---|---|27| 🔴 | `bug` | сломается или уже сломано |28| 🟡 | `risk` | работает сейчас, развалится при изменении условий |29| 🔵 | `nit` | стиль, именование, читаемость |30| ❓ | `q` | непонятно намерение, нужен ответ автора |3132Последней строкой итог: `totals: 1 bug, 2 risk, 3 nit, 1 q`.3334Нет находок — ответ `No issues.` целиком. Пустое ревью лучше выдуманного.3536## Что смотреть и в каком порядке3738Порядок отражает цену ошибки:39401. **Корректность** — делает ли код то, что заявлено412. **Границы** — пустые данные, нули, переполнения, отсутствующие ключи423. **Ошибки** — что происходит на неуспешной ветке, не проглатывается ли исключение434. **Конкурентность** — гонки, дедлоки, общее состояние445. **Безопасность** — данные из недоверенного источника, секреты, права456. **Читаемость** — имена, длина функций, комментарии4647Отдельно ищи то, чего в diff не видно: сломанные инварианты, места, где новый код противоречит соседнему, обработчики, которые надо было обновить вместе с этим.4849## Где формат разворачивается5051Правило `../../rules/agentops-auto-clarity.md` применимо и здесь. Однострочный формат выключается, когда находка касается:5253- **безопасности** — уязвимость объясняется целиком, с вектором и последствиями54- **архитектурного спора** — если проблема в подходе, а не в строке, одной строкой её не сформулировать55- **онбординга** — когда автор новичок и ему нужна не пометка, а объяснение5657В этих случаях после однострочной находки идёт развёрнутый абзац. Формат — инструмент экономии, а не самоцель.5859## Чего не делать6061- Не хвали. «Хорошая декомпозиция» не меняет ни строки кода.62- Не пересказывай изменения. Автор их написал.63- Не помечай `bug` то, в чём не уверен: есть `risk` и `q`.64- Не предлагай переписать всё иначе — для этого отдельный разговор, а не комментарий к строке.65- Не собирай находки ради непустого списка.66- Не ставь `nit` там, где в проекте нет соответствующего соглашения: это твой вкус, а не правило.
jtprogru