Інженерна майстерність і карʼєра / крок 08
Як покращити Code Review? Як це робить Google?
Про що це відео
Єдиного «правильного» способу робити код-рев'ю не існує — тож він ділиться досвідом з перших рук із двох місць: своєї компанії webilab і Google (де він працює). Він розбирає класичне рев'ю pull request-ів і те, як розвантажити його автоматизацією, гуглівську систему peer review + readability, те, як бути добрим автором PR, а потім — частину, яку, на його думку, недооцінюють: чому рев'ю окремих PR недостатньо, і процес періодичного архітектурного рев'ю, який webilab проводить, щоб ловити технічний борг, що накопичується, навіть коли кожен окремий PR виглядає чудово.
Головне
- Класичний потік (гілка → PR → рев'ю сеньйора → merge) працює, але лише з гайдлайнами: часто обидва підходи правильні, і треба просто стандартизувати один, щоб код усієї компанії читався однаково. Щойно рев'юер починає писати «тут 2 пробіли, там 4» — ви програли: автоматизуйте стиль геть за допомогою Prettier і ESLint з усіма плагінами, які лише знайдете, налаштованими під ваш стайлгайд. webilab навіть опублікувала власний плагін (
eslint-plugin-moreв npm) і додавала правила для проблем, що повторювалися на рев'ю, — щоб жодному сеньйору більше ніколи не довелося коментувати їх руками. - Хто рев'юїть? Рев'ю сеньйора/тімліда і peer review — працює і те, і те; залежить від того, чи є у вас узагалі вільні сеньйори. У Google — peer review: він може поставити рев'юером будь-якого гуглера, включно з джуном, що рев'юїть сеньйора.
- Гуглівська фішка — статус «readability»: окрім інженера, який знає фічу і каже LGTM, хтось із readability для цієї мови має засвідчити, що код відповідає стайлгайдам. Readability у TypeScript заробляється приблизно за пів року; у C++ — можливо, роками. У його команді джуніор має readability і може апрувити його код — тоді як його власного апруву самого по собі ніколи не досить, бо статусу він не має. Обидві ролі може поєднувати одна людина; командам, що змінюють мовний стек, призначають зовнішнього readability-рев'юера з будь-якого куточка Google.
- Google підпирає це колосальним обсягом статичного аналізу (під кожен тип файлів, нарощеним роками) і настільки однорідним монорепо, що «Stack Overflow практично не потрібен — користуєшся code search», знаходиш добрі приклади і вивчаєш будь-яку бібліотеку з реального використання.
- Будьте добрі до свого рев'юера — менталітет, який, за його словами, чудово працює в Google: у рев'юера є власна робота, тож зробіть рев'ю легким, якщо хочете, щоб воно було швидким, ретельним і повторюваним. Один merge request = одна тема; опишіть її; а до будь-якої зміни UI додайте скриншот чи скрінкаст, часто з позначками «було так → стало так». Рев'юер бачить поведінку миттєво.
- Фраза з книги, що запала йому в пам'ять: «якщо ти просто пишеш код — ти накопичуєш технічний борг». Його приклад: кодова база, спроектована під 10 модулів, тепер має 50 чи 300; кожен новий модуль пишеться і рев'юїться точно як попередні, кожен шматок виглядає чудово — а система вже не працює як задумано, і нічого з цього не видно в дифі перед мержем (технічний борг).
- Відповідь webilab: періодичне архітектурне рев'ю (щомісяця або раз на кілька місяців). Чеклісти типових фронтендних, бекендних і безпекових помилок (аж до речей на кшталт роботи з конфігами чи антипатерну анемічної доменної моделі); рев'юер спершу працює незалежно — починаючи із запуску проєкту, який має стартувати однією командою; команда самоаудитується за тим самим чеклістом; потім ~2-годинний дзвінок, на якому тімлід спершу малює C4-діаграму контейнерів (нотацію можна вивчити за 5 хвилин) процесів/підсистем; на виході — документ і план дій; повторне рев'ю через ~2 місяці (проектування ПЗ).
- Рев'ю після запуску в Google: коли щось виїжджає в продакшен, команда збирається, і той, хто реалізував велику фічу, проводить усіх по коду і відповідає на запитання — вже після мержа. Його підсумок: не *недо*оцінюйте ці інші типи рев'ю і не *пере*оцінюйте рев'ю на merge request-ах, бо самого по собі його просто недостатньо.