Спробую пояснити на своєму досвіді проведення code review. Сподіваюсь, мої поради дозволять вам побудувати ефективну комунікацію з підопічними.
Code Review — навіщо взагалі потрібен?
Код рев’ю — звична практика не тільки на початку роботи. Часто через це проходять і досвідчені фахівці. Та саме початківці інколи не розуміють, навіщо комусь передивлятися їх код. Відсутність код рев’ю може призвести до багів не лише у фрагменті коду, який він писав. Проблема може виникнути й в інших частинах проєкту. Тому ментору слід аргументовано донести джуну правила кодингу. Експерт має спиратися не лише на власний досвід, але й на технічну документацію.
Взагалі-то робота будь-якого розробника повинна починатись із читання документації мови, з якою він працює, та опанування кодстайлу. Наприклад, є Google TypeScript Style Guide з описом правил кодингу в TypeScript.
Головний акцент під час код рев’ю робиться на читабельності коду. Чистий код спрощує його подальшу підтримку як на вашому боці, так і на стороні замовника. Адже відповідно до життєвого циклу проєкту, він може перейти до іншої команди. І тоді вже інші девелопери будуть розбиратися з вашим кодом, додавати нові фічі чи розв’язувати знайдені проблеми.
Скільки часу треба приділяти код рев’ю?
Я проводжу код рев’ю з кожної нової фічі чи змін, які вносять молодші колеги згідно із системами контролю версій. Усюди свої терміни імплементації тих чи інших змін. Тому інколи займаюсь цим щодня, інколи — раз на тиждень. Взаємодію з джуніором будую так, щоб після виконання таску він повідомив мене про необхідність код рев’ю. Так я вкотре перевіряю правильність його дій. Жодні зміни не додаються в проєкт без апруву з мого боку. Хоча іноді код рев’ю охоплює всі аспекти проєкту. Але здебільшого це відбувається в зовнішніх проєктах, де потрібен сторонній погляд експерта з певної технології.
Щодо часу, то тут все залежить від обсягу фічі та якості коду. На моїй пам’яті були ситуації, коли на код рев’ю заходило по 300-500 файлів… У такий момент навіть не хочеться відкривати лінк. А коли маєш декілька файлів, можеш одразу їх перевірити за короткий період.
Ми в команді сформувати такий підхід: щодня з 10:00 до 12:00 займаємося пул реквестами. Якщо встигнемо закінчити раніше, повертаємось до основної роботи. Раптом бачимо, що потрібно більше часу — переносимо на наступний день. Підхід не ідеальний, звісно. Інколи люди не мають часу і пропускають щось. Але ця модель гарно працює як база. Інакше очікування на код рев’ю та мердж в основну гілку розтягнеться на тижні.
Зазвичай для виправлення я не ставлю ліміти часу. Після передачі коду на рев’ю розробник може переключитися на інші задачі. На обробку об'ємного пул реквеста піде чимало часу і в мене, і у виконавця. Жорсткі рамки будуть недоречними. Людина відволікатиметься від основної задачі, не зможе зосередитись. Тому ліпше напишіть так: «Залишив коментарі, подивись у вільний час. Якщо будуть питання, я відповім».
Проте й тут є ризики. Пам’ятаю, як розробник після мого повідомлення тиждень не реагував на пул реквести. Тож уточнюйте їх статус. Особливо у початківців, які зазвичай не навантажені тасками. Після код рев’ю для них найбільш логічно пройтись по всім коментарям. Якщо здається, що людина згаяла час, зверніться до керівника проєкту або проджект-менеджера.
Основні етапи Code Review
Усе починається зі знайомства з репозиторієм проєкту. Так ми економимо час, аби не відкривати кожен окремий файл. Ідеальна ситуація: коли вдається запустити проєкт без додаткових пояснень розробника. Далі я можу самостійно аналізувати потрібні області проєкту, вивчати архітектуру, перевіряти дотримання принципів програмування на кшталт Don’t Repeat Yourself або Keep It Simple. Якщо проєкт не запускається, намагаюсь дізнатись причину разом із розробником.
Наступний крок можна назвати спробою передбачити майбутнє. Я не просто шукаю помилки, а розмірковую, чи може такий стиль написання коду призвести до негативних наслідків надалі. На цьому етапі можна оцінити, наприклад, правильність використання фреймворків та бібліотек. Вони можуть впливати на розмір проєкту в процесі білду застосунку. Хоча наразі в коді немає помилок, та в майбутньому через обрану джуном бібліотеку, проєкт ставатиме більшим, ніж за іншої реалізації. Це вже питання оптимізації та рефакторингу.
Також варто оцінити кодстайл розробника. Тут багато суб’єктивного, тому краще посилатися на документацію і приклади з власної практики. Все треба збирати в пул реквестах, систематизувати й обговорювати. Обов’язково послухайте аргументи — чому розробник застосував певне рішення. Ніколи не треба просто засуджувати. Передусім ви як ментор повинні дати позитивний відгук, підкреслити гарні сторони коду. Я особисто вказую, що мені сподобалося в коді, які саме частинки. Роблю це за допомогою коментарів — Looks Good To Me. І хоча такі моменти здаються дрібницями, але все ж таки будуть приємні кожному. Особливо важливо чути щось позитивне початківцям. Хороші слова допомагають їм не втрачати мотивацію і не зосереджуватись лише на негативі у пул реквестах.
Які коментарі залишати?
Треба розрізняти дійсно необхідні та просто бажані коментарі. Передусім я зосереджуюсь на виправленнях, без яких не обійтися. Та якщо бачу певні рішення в коді, які я зробив би інакше, залишаю коментар як пораду. Щось на кшталт: «Це краще реалізувати інакше, але те, що зробив, не критично». Нехай буде на розсуд самої людини. Але якщо хочете, щоб ваша порада спрацювала на користь, вона має бути аргументованою. В іншому разі розробник майже завжди залишає все, як є.
Код рев’ю важливо робити як письмово, так і коментувати в розмові. На різних етапах перевірки коду вдаються до того іншого різновиду комунікації. Починаємо з письмових коментарів у пул реквесті. Зідзвони або зустрічі потребують час, тому базові питання краще вирішувати в переписці. Експерт у вільний час подивиться код та залишить поради. Розробник отримає на пошту повідомлення про нові коментарі до файлу і потім повернеться туди й все виправить. Однак тут є ризик пропустити щось важливе. На проєкті з великою командою подібні листи можуть підсвідомо сприйматися як спам. Та й фахівець може не встигати передивлятися їх. Раджу маякнути команді в робочому чаті, що код рев’ю закінчили, і тегнути відповідального за код. Або одразу пишіть йому особисте повідомлення.
Текстових коментарів інколи буває замало. Адже з вашою думкою в певних моментах можуть не погодитись. Тоді потрібно дискутувати. Почати можна і в треді, хоча на практиці зазвичай складно змістовно обговорювати щось у такому форматі. Як мінімум, можуть бути тривалі паузи в спілкуванні. А отже зміни в гілки Master чи Develop можуть залити запізно. Набагато ефективніше обговорювати коментарі до код рев’ю на дзвінку або зустрічі. Так, доведеться синхронізувати ваші графіки, але так буде простіше розв’язувати спірні питання.
Типові помилки джунів, які я виявив під час код рев’ю
- Недотримання стандартів кодування.
Як наслідок — нечитабельний код, складнощі у підтримці проєкту та інші труднощі в комунікації з командою.
- Неуважність до деталей.
Початківці роблять зайві пробіли та рядки або забувають поставити в кінці операції крапку з комою.
- Орфографічні помилки.
Це характерно не тільки для джунів. Досвідчені розробники теж можуть неправильно писати англійською назви функцій, змінних або класів.
Наприклад, є поле Disabled — неактивне. Але можна ж написати Is Disabled — є неактивним. Це уточнення зберігатиметься в змінній, і надалі тут можуть виникати труднощі при виконанні програми чи її білді. Дехто думає: якщо щось не заборонено правилами проєкту, можна і так зробити, навіть якщо це буде помилкою. Мушу не погодитись із цим. Насправді далеко не все можна обробити правилами, плагінами, лінтерами, файлами конфігурації тощо.
- Помилки в системі контролю версій.
Девелопер може викликати якусь команду в терміналі випадково, через що дані видаляться чи відправляться до репозиторію на сервері. Також новачки помилково маніпулюють гілками в Git, зокрема й після код рев’ю. Тоді зміни не відображаються в програмі, з’являються конфлікти, а код не виконується. Хоча помилка може бути не помітною. Тобто система повернулась до попереднього стану, і конфлікти неочевидні. Побачити, що щось пішло не так, вдасться лише під час мануального тестування коду.
- Нехтування тестуванням коду.
Початківці не люблять перевіряти свій код на відповідність вимогам. Це може призвести до багів у роботі програми.
- Неоптимізований код.
Часто джуни не дбають і про оптимізацію власного коду. У результаті може погіршитися продуктивність програми, знизитись її ефективність.
- Неправильне використання бібліотек і фреймворків.
Так виникають проблеми з безпекою та ефективністю програми загалом.
Джуни бувають доволі чутливими, їх легко демотивувати своїми негативними коментами. Головне не стільки кількість і характер коментарів, скільки ваше людське ставлення до молодшого колеги. Будьте ввічливими у своїх зауваженнях, поважайте людину за виконану роботу, якої б якості вона не була. Усе-таки всі ми колись були джунами.
У продовженні цієї теми в наступній статті я розповім, як балансувати між негативними та позитивними коментарями під час код рев’ю.