Пошук уроків, статей та іншого контенту
Навчитеся читати коментарі, пропонувати зміни, відповідати на зауваження та завершувати перевірку коду.
Code Review — це перевірка змін у Pull Request (PR) перед їхнім об’єднанням у цільову гілку. Рев’юер аналізує не лише синтаксис коду, а й:
відповідність вимогам задачі;
коректність алгоритму;
обробку помилок і граничних випадків;
безпеку;
продуктивність;
читабельність і підтримуваність;
достатність тестів;
сумісність із наявною архітектурою.
Pull Request — це не просто запит «перевірте мій код». Це простір для обговорення змін, у якому кожен коментар має допомагати покращити код або уточнити рішення.
Перед детальним переглядом варто сформувати загальне уявлення про зміни.
Перевірте:
яку проблему вирішує PR;
який очікуваний результат;
чи є додаткові обмеження;
як протестувати зміни;
чи пов’язаний PR із конкретною задачею;
які частини автор навмисно залишив поза межами PR.
Якщо опис не пояснює мету змін, спочатку поставте уточнювальне питання. Інакше можна витратити час на перевірку коду, який вирішує не ту проблему.
Зверніть увагу на:
кількість змінених файлів;
випадково додані файли;
зміни форматування, не пов’язані із задачею;
зміни конфігурації;
міграції бази даних;
зміни API або публічних контрактів;
видалення тестів.
Великі PR важче якісно перевіряти. Якщо зміни містять кілька незалежних задач, доречно попросити автора розділити їх на окремі PR.
Спочатку перегляньте зміни на високому рівні:
де починається новий сценарій;
які функції викликаються далі;
де перевіряються вхідні дані;
де обробляються помилки;
як формується результат;
які побічні ефекти виникають.
Лише після цього переходьте до окремих рядків. Так локальний коментар буде пов’язаний із поведінкою всієї системи.
Зручно проводити review у кілька проходів.
Перевірте, чи реалізовано саме потрібну поведінку:
чи враховано всі сценарії з задачі;
що відбувається для порожніх даних;
що відбувається для некоректних даних;
чи не змінилася поведінка існуючих сценаріїв;
чи відповідають повідомлення про помилки вимогам.
Проаналізуйте:
межі відповідальності функцій і модулів;
напрямок передачі даних;
повторне використання логіки;
побічні ефекти;
роботу зі станом;
узгодженість із наявними патернами проєкту.
На цьому етапі краще залишати загальні коментарі до фрагмента або файлу, а не десятки зауважень до кожного рядка.
Зверніть увагу на:
відсутність перевірки вхідних даних;
витік внутрішніх помилок користувачу;
обхід авторизації;
небезпечну роботу з SQL, HTML або командним рядком;
неправильне поводження з секретами;
умови гонки;
повторне виконання операцій;
обробку тайм-аутів і частково виконаних операцій.
Перевірте:
чи покривають тести нову поведінку;
чи є тести для граничних випадків;
чи тестують помилки, а не лише успішний сценарій;
чи не залежать тести від випадкового порядку або реального часу;
чи зрозумілі назви;
чи немає зайвої складності;
чи не приховує короткий код важливу логіку.
Не всі зауваження мають однакову важливість. Це потрібно явно позначати, щоб автор правильно визначив порядок роботи.
Таке зауваження вказує на проблему, через яку зміни не можна безпечно об’єднувати:
неправильний результат;
втрата або пошкодження даних;
вразливість;
падіння в типовому сценарії;
порушення публічного контракту;
відсутність обов’язкової перевірки.
Формулюйте причину конкретно:
Якщо
userIdне знайдено,userмає значенняundefined, а звернення доuser.emailзавершується винятком. Потрібно обробити цей випадок до доступу до властивості.
Це покращення, яке не повинно затримувати об’єднання, якщо команда не домовилася інакше:
назва може бути точнішою;
код можна спростити;
варто додати пояснення до складного алгоритму;
можна використати вже наявну утиліту.
Для таких коментарів корисно використовувати позначки на кшталт nit або non-blocking, якщо це прийнято в команді.
nit: Назваdataне пояснює призначення значення.normalizedEmailточніше передає його роль. Це не блокує PR.
Питання не завжди означає вимогу змінити код. Воно допомагає з’ясувати намір автора:
Чи потрібно тут зберігати порядок елементів? Якщо так, поточний підхід із
Setможе бути недостатнім для цього контракту.
Не маскуйте твердження під питання. Фраза «Чи можна додати перевірку доступу?» може звучати необов’язково, хоча перевірка є критичною.
Пропозиція містить конкретний напрямок покращення:
Можна виконати перевірку
items.lengthдо створення транзакції, щоб не відкривати транзакцію для порожнього запиту.
Пропозиція має залишати автору простір для вибору реалізації, якщо проблема не є блокувальною.
Хороший коментар має описувати:
де саме проблема;
чому вона важлива;
який сценарій її виявляє;
що потрібно змінити або уточнити.
Порівняйте:
Це неправильно.
і:
Цей запит використовує значення
sortбез перевірки зі списком дозволених полів. Користувач може передати невідоме поле, а в деяких драйверах — вплинути на сформований запит. Перевірте значення через whitelist перед побудовою запиту.
Невдалі формулювання:
«Ти не розумієш, як це працює».
«Це очевидно треба переписати».
«Жахливе рішення».
«Чому ти взагалі так зробив?»
Кращі формулювання:
«Цей підхід не враховує повторний виклик функції».
«Тут виникає проблема, якщо відповідь сервера має статус 204».
«Чи можемо використати наявний сервіс, щоб зберегти єдину перевірку прав доступу?»
Якщо проблема стосується лише форматування, назви або особистої переваги, не варто блокувати PR. Автоматичний formatter і linter мають перевіряти механічні правила до початку ручного review.
Багато платформ підтримують пропозиції змін безпосередньо в коментарі до рядка. Такий механізм зручний, коли заміна локальна й однозначна.
Наприклад, замість коментаря:
Тут краще повернути порожній масив, щоб споживачі не отримували
null.
можна запропонувати конкретну заміну:
return [];Автор може застосувати пропозицію, після чого платформа створить коміт або внесе зміну відповідно до налаштувань репозиторію.
Пропозиції варто використовувати, якщо:
зміна невелика;
її правильність очевидна;
вона не приховує важливого архітектурного рішення.
Не використовуйте пропозицію для великого рефакторингу або зміни поведінки без пояснення. У такому випадку спочатку опишіть проблему й обговоріть рішення.
Розглянемо функцію, яка розраховує підсумкову суму замовлення.
Початкова реалізація:
export function calculateTotal(items, discount = 0) {
const subtotal = items.reduce((sum, item) => {
return sum + item.price * item.quantity;
}, 0);
return subtotal - discount;
}Проблеми:
не перевіряється, що items є масивом;
кількість товару може бути від’ємною або нецілою;
ціна може бути від’ємною;
знижка може перевищувати суму;
немає перевірки структури елемента.
Можливий коментар до PR:
priceіquantityвикористовуються без перевірки. Від’ємна кількість дозволить зменшити суму замовлення, аNaNможе непомітно поширитися до відповіді API. Додайте валідацію елементів або використайте вже наявний валідатор замовлення. Це блокує PR, оскільки значення надходять із зовнішнього запиту.
Один із варіантів виправлення:
export function calculateTotal(items, discount = 0) {
if (!Array.isArray(items)) {
throw new TypeError('items має бути масивом');
}
if (!Number.isFinite(discount) || discount < 0) {
throw new RangeError('Знижка має бути невід’ємним числом');
}
const subtotal = items.reduce((sum, item) => {
if (
!Number.isFinite(item.price) ||
item.price < 0 ||
!Number.isInteger(item.quantity) ||
item.quantity < 0
) {
throw new RangeError('Товар має некоректні ціну або кількість');
}
return sum + item.price * item.quantity;
}, 0);
return Math.max(0, subtotal - discount);
}Такий приклад можна перевірити локально:
import assert from 'node:assert/strict';
import { calculateTotal } from './calculate-total.js';
assert.equal(
calculateTotal(
[
{ price: 100, quantity: 2 },
{ price: 50, quantity: 1 },
],
30,
),
220,
);
assert.equal(calculateTotal([], 10), 0);
assert.throws(
() => calculateTotal([{ price: 100, quantity: -1 }]),
RangeError,
);
console.log('Усі перевірки пройдено');Під час review важливо не просто запропонувати «додати валідацію», а перевірити, чи відповідає її поведінка контракту системи. Наприклад, команда може домовитися повертати помилку валідації замість кидання винятку. У такому разі коментар має посилатися на цей контракт.
Автор PR має опрацювати кожне змістовне зауваження, а не лише внести зміни мовчки.
Хороша відповідь містить:
що саме зроблено;
де це змінено;
як перевірено результат;
пояснення, якщо пропозицію не застосовано.
Приклади:
Виправлено: додав перевірку
userIdперед зверненням до профілю. Додав тест для випадку, коли користувача не знайдено.
Не застосовано навмисно. Тут потрібен
null, а не порожній масив, оскільки клієнт розрізняє «даних немає» і «пошук не виконувався». Додав це до опису API.
Змінив на спільний валідатор
validateOrder, щоб не дублювати правила. Тести для невалідної ціни й кількості пройдено.
Не варто відповідати лише «done», якщо з відповіді незрозуміло, що саме змінилося. Також не слід позначати коментар вирішеним, якщо проблема не виправлена й немає узгодженого пояснення.
Розбіжності під час review нормальні. Відмова від пропозиції має стосуватися технічних причин, а не статусу учасників.
Структура відповіді:
підтвердити, що пропозицію зрозуміло;
пояснити обмеження;
навести альтернативу або докази;
за потреби запропонувати обговорення з відповідальним за компонент.
Приклад:
Розумію пропозицію перенести перевірку в middleware. У цьому PR endpoint також викликається внутрішнім job-обробником, який не проходить через цей middleware. Тому залишаю спільну перевірку в сервісному шарі й додав тест для обох шляхів виклику.
Якщо питання стосується архітектурного рішення і сторони не можуть дійти згоди, не варто продовжувати довгу дискусію в одному рядку коду. Краще винести питання в окреме обговорення команди, зафіксувавши його результат у PR.
Використовуйте його, коли проблема прив’язана до конкретного рядка або невеликого фрагмента:
помилка в умові;
відсутня перевірка;
неправильний виклик функції;
неочевидна операція.
Використовуйте його для питань, які стосуються всієї зміни:
PR не відповідає заявленому обсягу;
відсутня стратегія міграції;
зміни конфігурації потребують окремого узгодження;
загальна структура ускладнює подальше розширення;
потрібно додати опис способу тестування.
Не дублюйте загальним коментарем те, що вже детально пояснено в інлайн-коментарях.
Назви можуть відрізнятися залежно від платформи, але зазвичай є три основні результати.
Ви перевірили зміни й не маєте блокувальних зауважень. Це не означає, що код ідеальний або що ви гарантуєте відсутність будь-яких помилок.
Перед схваленням переконайтеся, що:
усі критичні коментарі закриті;
автоматичні перевірки успішні;
зміни відповідають задачі;
ви переглянули актуальний стан PR після останніх змін.
Застосовується, коли є проблема, яку потрібно виправити до об’єднання. Використовуйте цей стан лише для обґрунтованих блокерів.
У коментарі до такого review чітко вкажіть:
що блокує об’єднання;
чому це небезпечно або некоректно;
який результат очікується.
Застосовується, коли ви залишаєте питання або рекомендації, але не блокуєте PR.
Наприклад:
потрібно уточнити намір;
є неблокувальне покращення;
рішення прийнятне, але варто зафіксувати ризик;
ви хочете дати зворотний зв’язок без формального схвалення.
Перед завершенням review виконайте фінальну перевірку:
Перечитайте опис PR і переконайтеся, що вимоги виконано.
Перевірте змінені файли повністю, а не лише окремі рядки.
Переконайтеся, що всі блокувальні коментарі мають відповідь.
Перевірте, що нові коміти не створили додаткових проблем.
Перегляньте результати тестів, linter і збірки.
Переконайтеся, що актуальна версія гілки є тією, яку ви перевіряли.
Виберіть правильний стан review.
Якщо після вашого review автор додав значні зміни, перегляньте пов’язані ділянки повторно. Не покладайтеся лише на попередній висновок: нове виправлення може вплинути на інший сценарій.
Для складних змін корисно отримати гілку PR локально й запустити тести.
Типова послідовність:
# Отримати актуальний стан віддаленого репозиторію
git fetch origin
# Перейти на локальну гілку PR
git switch feature/order-total
# Оновити її без створення зайвого merge-коміту
git pull --ff-only origin feature/order-total
# Переглянути різницю з цільовою гілкою
git diff origin/main...HEAD
# Переглянути список змінених файлів
git diff --stat origin/main...HEAD
# Перевірити історію комітів PR
git log --oneline --decorate origin/main..HEAD
# Запустити перевірки проєкту
npm test
npm run lintТри крапки в git diff origin/main...HEAD показують зміни гілки відносно спільного предка з main. Це зазвичай відповідає змінам Pull Request точніше, ніж порівняння двох поточних вершин після складної історії злиттів.
Якщо PR оновився, повторіть git fetch, перевірте актуальність локальної гілки й за потреби повторіть ключові сценарії.
Один інлайн-коментар може породити кілька відповідей. Щоб обговорення залишалося зрозумілим:
відповідайте на конкретне питання;
не змінюйте тему посеред ланцюжка;
після виправлення коротко опишіть результат;
закривайте thread лише тоді, коли питання вирішене;
якщо рішення змінилося, оновіть висновок у фінальному коментарі.
Якщо коментар більше не актуальний через зміну коду, це не завжди означає, що проблема вирішена. Перевірте, чи новий код справді усуває початковий ризик.
Не кожна відмінність від вашого стилю є помилкою. Спирайтеся на вимоги проєкту, документацію, вимірюваний ризик або узгоджені правила.
Фраза «додайте тест» недостатня. Вкажіть, який сценарій не покрито і яку поведінку потрібно перевірити.
Окремий рядок може виглядати правильно, але бути несумісним із викликачами, типами даних або транзакційним потоком. Переглядайте контекст і пов’язаний код.
Якщо всі коментарі звучать однаково терміново, автору складно визначити пріоритети. Явно розділяйте обов’язкові виправлення, питання та неблокувальні пропозиції.
Після нових комітів попереднє схвалення може вже не відображати стан коду. Перевіряйте зміни після суттєвих оновлень.
Закриття коментаря не замінює перевірку. Переконайтеся, що зміна справді усуває проблему й не створює нову.
Рев’юер не має без потреби перехоплювати реалізацію. Якщо проблема не є однозначною, спочатку поясніть ризик і запропонуйте напрямок, а не повний rewrite.
Критикуйте рішення, поведінку коду та ризики. Професійний тон робить складні технічні дискусії продуктивними.
Починайте review з опису PR і загального потоку змін.
Перевіряйте вимоги, коректність, помилки, безпеку, тести й підтримуваність.
Розділяйте блокувальні зауваження, питання та неблокувальні пропозиції.
Формулюйте коментарі конкретно: проблема, наслідок, сценарій і очікувана дія.
Використовуйте code suggestions лише для невеликих однозначних замін.
Відповідайте на кожен коментар і пояснюйте, що було змінено або чому пропозицію не застосовано.
Не плутайте особисті вподобання з дефектами.
Перед Approve або Request changes перевіряйте актуальний стан PR і результати автоматичних перевірок.
Закривайте обговорення лише після фактичного вирішення питання.