Code review. Які є основні правила перевірки коду
Я — Євгеній Вінійчук, у розробці вже понад 7 років. У компанії Youshido виріс від Trainee до Lead Front-end Developer. Створював сайти та застосунки як для стартапів, так і для бізнес-гігантів — Viasat, Jacobs, Pepsi, Tuborg тощо. Зараз поєдную функції тімліда та Senior Software Engineer. У цьому матеріалі розкажу про власний досвід code review, як ми його організували в Youshido, які правила я засвоїв і на які помилки звертаю увагу.
І спочатку був код...
Додати кнопку на сайт, замовити зворотний дзвінок або розробити новий розділ застосунку — для кожного з цих завдань розробники пишуть код окремо, подібно до гілок, які відростають від стовбура дерева. Щоб бути впевненим у правильності виконаної роботи, розробники перевіряють код одне одного під час code review.
Спочатку в Youshido не було цього процесу. По-перше, кожен виконував свою роботу якісно, адже ядром команди були досвідчені розробники. Коли я тільки починав там працювати у свої 18 років, для мене їхній рівень видавався космосом і справляв вау-ефект. По-друге, ми мало багато паралельних проєктів, життєвий цикл яких був короткий — один-два місяці. Тому на рев’ю не вистачало часу, та й на якість роботи це не впливало.
Та з часом, коли команда почала розростатися, коли прийшли нові стажери й джуніори, які потребували фідбеку, з’явилась потреба в повноцінному code review. Друга причина: ми були зацікавлені в тому, щоб «виростити» під себе команду. А зростання не може відбуватися без повноцінного діалогу та рецензування роботи.
Тож завдання просто протестувати код розрослося до низки інших:
- освіта (розуміння архітектури проєкту, навчання джунів);
- обмін досвідом (цілком можливі варіанти, коли джун рев’ювить сеньйора або два сеньйори проглядають код одне одного — так вони не «варяться у власному соку», а прогресують);
- пошук кращих рішень і постійне удосконалення кодової бази.
А далі було review
Механізм code review достатньо простий: написали код — перевірили — внесли правки — протестували. Наприклад, з’явилося завдання додати кнопку на сайт. Воно закріплюється за певним розробником. Він створює собі гілку від основного коду, тобто паралельну історію змін. Там пише все, що потрібно для появи кнопки на сайті. Коли робота виконана, розробник створює merge request, де вказує своїх рев’юерів.
Той, хто перевіряє код, бачить пофайлово всі зміни й може писати свої зауваження та поради під кожним рядком коду. Понад те, проаналізувавши готову роботу, відтворює для себе послідовність і логічність дій розробника, тобто чи буде це працювати. Потім усі коментарі відсилають назад до виконавця задачі. Така процедура може повторюватися декілька разів, поки рев’юер остаточно не прийме написаний код. Коли він прийнятий, його поєднують з основним кодом проєкту. Після цього оновлюється поточна версія застосунку, а далі завдання переводиться на QA.
Ніби нічого особливого. Але, як завжди, вся суть у деталях. І я виписав собі деякі зауваги, на які зважаю в роботі.
Як робити не треба, або «Вийди і зайди нормально»
Не змішувати стиль коду, бізнес-логіку та рефакторинг. У мене бували випадки, коли у файлі, який давно не редагували, розробник разом з виконанням задачі міг відформатувати стиль коду. Наприклад, усі подвійні лапки перетворити в одинарні чи поміняти форматування коду. У результаті мені приходив файл зі змінами у 80% рядків, тоді як треба було просто змінити назву розділу (1 рядок). Це типове змішування задач різного типу, яке змушує вишукувати зміни та не дає сфокусуватись. (Найімовірніше, я одразу поверну такий реквест із проханням розділити зміни.)
Маленькі реквести завжди кращі за великі. Якщо змінилися 10 рядків коду, в них можна знайти 10 помилок. А якщо зміни внесли одразу в 1000 рядків, то «загалом все окей». Навіть якщо на початку мозок вчитується і намагається аналізувати великий масив інформації, то ближче до середини уже сам рев’юер може припускатися помилок. Також з великими завданнями важче простежити логіку та послідовність дій розробника, а на перевірку та написання коментарів можуть піти години. Тому для ефективної та якісної роботи великий обсяг треба дробити на менший.
Усе, що було змінене, має бути протестоване. Інколи хочеться виправити якусь просту частину й не проходити довгий і важкий шлях тестування знову, адже «та це ж відступ поправили». Я багато разів наступав на ці граблі, коли зайві 10 пікселів зліва ламали весь блок. Але застосунок стає складніший, водночас збільшується кількість потенційних помилок. Тому завжди дешевше перевірити власноруч, ніж потім писати хотфікс.
Не забувайте, навіщо ви це робите in the first place. Дуже круто, коли компанія може витрачати на code review стільки часу, скільки потрібно, а розробники можуть дійти до найкращого рішення. Легко впасти в азарт під час пошуку ідеального підходу й забути про те, що основна функція, яку виконує розробник, — це вирішення завдань бізнесу. Якщо часу обмаль, а задача потрібна «на вчора», то треба забезпечити її розв’язання якнайшвидше. В такому випадку я пропоную приходити до good enough рішення, яке може «прожити» певний час, а паралельно з ним створювати задачу на закриття технічного боргу.
Він з’являється тоді, коли ми починаємо «зрізати кути», бо розробнику треба якнайшвидше виконати завдання або проєкт не готовий до нових бізнес-потреб. Але після виконання завдання до цих частин обов’язково треба повернутися, інакше проєкт буде важко розвивати й з’являться помилки в тих місцях, де ніхто не очікував. Щоб не забути, що саме треба виправити, раджу вести перелік таких місць. Найефективнішим засобом для мене виявилось розставляти коментарі @todo, які потім можна знайти й уже все доробити. До того ж IDE надає зручний функціонал з пошуку саме @todo.
Критикуй без фанатизму, бо лишишся без джуніора
Не варто забувати про те, що code review — це оцінювання та критика чужої роботи. І для збереження здорових відносин у колективі та для мотивування колег варто робити це екологічно. В мене є такі основні правила:
1. Висловлювати свою критику не щодо людини, а щодо її роботи. Не «ти/твій», а «цей підхід/таке рішення можна покращити так-то». Такий принцип прибирає персоналізацію, і розмова ведеться в професійному ключі. Ви починаєте «грати в одній команді», шукаючи кращий підхід до розв’язання задачі.
Цей принцип я виробив на власному досвіді. На початку своєї кар’єри я особливо переймався через якість виконаної роботи. До того ж мені бракувало базових знань, що лише підсилювало почуття тривоги, адже хотілося бути на рівні з усіма й не відчувати себе «не таким». В мене були різні ментори, що дало можливість відчути різницю в підходах.
Здебільшого, коли ми перевіряли код, саме завдяки тому, що розмова велася про певне рішення, а не про мій (персональний) код, я заспокоювався і все більше довіряв ментору. Мені дали зрозуміти, що я можу отримати фідбек про роботу й ніхто не буде вважати мене поганою людиною, якщо рішення буде не найкращим.
Проте траплялося, що я отримував коментар «в лоб», тому я розумію, наскільки може демотивувати людину відгук у стилі «все, що ти зробив — не таке». Після цієї фрази впевненість згасає або, що гірше, через невпевненість і страх знову припуститися помилок і нарватися на критику спеціаліст факапить ще більше.
2. Вести активний діалог. У процесі перевірки коду дивлюся на написане з колегою, і ми разом думаємо, чи можна це рішення покращити. Ви граєте на одному рівні. Для того, щоб це не перетворилося на «роби, як я сказав», можна ставити питання на зразок «Як думаєш, чи можна це переписати краще?» й після відповіді співрозмовника запропонувати власну позицію, яка, найімовірніше, й буде прийнята.
Одного разу ми з колегою зідзвонилися для код-рев’ю, проте він не давав активного зворотного зв’язку. Без фідбеку з «тої сторони» складно підсумувати результат роботи. Бо в кінці ми разом маємо отримати план удосконалення поточного рішення або погодитись, що зміни не потрібні. Тоді я почав ставити питання, які допомогли розкритися, і діалог таки запустився. Виявилося, що у колеги не було досвіду схожих рев’ю, він не знав, як саме це має відбуватись. З часом він почав активно висловлювати свою думку, і робота пішла. Відтак проактивним діалогом вдалося розв’язати кілька завдань.
Така форма активного діалогу допомагає «витягнути» з молодих фахівців їхню думку, яку їм іноді непросто сформулювати. Через невпевненість у собі й відсутність значного досвіду людина може просто змовчати, навіть якщо не погоджується із запропонованим рішенням. Коли я наполегливо цікавився поглядом колег і допомагав висловитися, вони відкривалися. У результаті ми вбиваємо двох зайців — закриваємо задачу та підвищуємо мотивацію співробітника. Бо, коли людина розуміє, що її думка цінна, вона хоче зростати далі.
3. Після написання коментарів зідзвонитися особисто та все проговорити. Спілкування дасть чітко зрозуміти інтонацію вашої критики, що налаштує «приймач» людини на правильну хвилю. Адже інколи в письмовому вигляді критика може сприйнятися важче, ніж у розмові в стилі «Слухай, тут треба ось це й це змінити».
Також проговорювання вголос може допомогти розставити пріоритети й зрозуміти одне одного, чого не завжди вдається досягти листуванням. Якось ми обговорювали завдання — новий функціонал для редактора сайту. Я оцінив роботу в чотири години, а мій колега — у два дні. До цього ми начебто були на одній хвилі, але коли дійшло до оцінювання часу, стало зрозуміло, що один із нас щось не врахував. Якби ми не проговорили цей момент уголос, то в результаті завдання було б виконано зовсім не так, як того очікував клієнт.
4. Не критикою єдиною. Передусім завдання рев’юера — подивитись свіжим поглядом на виконану роботу й знайти кращі рішення. Але ще підбадьорити колегу за цікаве чи нестандартне рішення або за якісно виконану роботу. Коли я проводжу рев’ю завдання, яке виконав джуніор, завжди намагаюся сказати про те, що загалом все зроблено правильно та відповідає очікуваному результату. Звертаю увагу на ті місця, де розробник вдосконалюється, та відзначаю їх під час рев’ю.
Під час код-рев’ю виявляється чимало помилок, які можна виправити, зменшити кількість циклів back-and-forth при роботі з QA, а значить збільшити швидкість розробки. Окрім того, завдяки перевірці коду підвищується загальний рівень знань про проєкт, що зменшує так званий bus factor.
Також спільний перегляд і редагування коду добре прокачує команду. Мені подобається разом шукати рішення, обговорювати завдання, бачити, як люди зростають. Я спостерігав за співробітниками, які проходили шлях від новачків, яким потрібно приділяти багато часу, до повністю самостійних юнітів, на яких ти можеш цілком розраховувати.
Під час код-рев’ю ви виховуєте команду мрії для себе, оскільки проводите разом багато часу й починаєте розуміти одне одного з пів слова. Для мене це не лише про технічні навички, а й про педагогіку, мотивацію та комфортність роботи в команді. Адже лише силою вмотивованої команди можна досягти значних успіхів у масштабі продукту, а не лише його маленької частини.
66 коментарів
Додати коментар Підписатись на коментаріВідписатись від коментарів