Як покращити Code Review? Як це робить Google?

Є різні підходи до Code Review. Ділюсь своїм досвідом Code Review в Google та інших компаніях
Станьте спонсором цього каналу: / @aboutprogramming
Допоможіть каналу розвиватися й отримуйте доступ до ексклюзивного контенту.
Зміст відео:
0:00 - Вступ
🏠 Мої соцмережі:
Жабаскрипт в телеграмі - t.me/jabascript
Я в Твітер - / viktorturskyi
Мій Linkedin - / turskyi
#programming #javascript #codereview #програмування #українською

[@romanyukartem757]

Надавно натрапив на ваш канал. Контент та подача - супер, дякую!

[@cruc1anberg]

Щастя здоровʼя тобі!!!!

[@AdminAdmin-sl2qf]

❤❤❤❤❤❤❤❤❤❤❤❤

[@atsybulsky]

Бест практіс великих компаній типа гугла - дуже цікаво =)

[@olegv5793]

Віктор, дякую за якісний контент! 👍 Було б цікаво дізнатись більше про те як працює зв‘язка PO і Tech Lead. А також про те як працює refinement задач у гуглі.

[@McKleeM]

Не встиг і моргнути, дякую)))

[@user-pt7nz6vc7s]

Дякую за відео 😊

[@-Oleg1]

Все по темі, без води, дякую

[@alexanonymous5823]

цікаво, дуже дякую

[@shchekavytsia]

Прикольні відео! Підписався.

[@artemtrush]

Мені подобається частота виходу відео ✨🐸 Як в гуглі з пріоритетністю рев'ю перед поточними задачами? Бо зазвичай рекомендують давати фідбек максимально швидко, щоб не блочити людину/фічі. І ще, десь бачив гарну фразу - Пишіть коментарі до коду, а не до людини, яка його написала. (politeness)

[@AboutProgramming]

Звісно краще не блочити людину, але й той, хто створює реквест теж має розуміти, що людина може зараз бути зайнята іншими справами. Якщо рев'ю в рамках одного дня, то це добре. Відносно того, як правильно давати (й сприймати) фідбек, то це окрема тема)

[@ivansepp7928]

Супер.

[@Vladyslav_Sliusar]

О, про анемік модель цікаво послухати)

[@AboutProgramming]

Обов'язково буде відео про це

[@Vladyslav_Sliusar]

@@AboutProgramming дяка

[@Bohdan-Venhrenovych]

Топчик

[@BelladonGameDeverUA]

Толково розповів Добре, що на ДОУ показали розмову і так я знайшов цей канал

[@user-lc7cz2ks6i]

👍👍👍👍

[@vandriichuk]

Для Гугла може і норм такий складний ревью, але ж безліч проектів, де це овер. Тому думаю, что все від проектів залежить. Для стартапу швидкість імплементації фічі набагато важливіша технічного боргу

[@average-user9]

Дякую, аж захотілося попрацювати в схожій компанії чи команді, зі схожими принципами... Було б цікаво почути ще, як відбувається процес постановки тасків/задач, від того як з'являється ідея, хто і як її розбиває на фічі і далі на таски

[@invilso4051]

Віктор у інтерв'ю на DOU відповідав на схоже питання, якщо коротко то фічі вигадують розробники, вони ж їх і рубають на менші частинки.

[@artemkonyukhov6635]

Сподобалось, що є ревью архітектури і є ревьювери з редабиліті статусами. Дякую.

[@eolit1o]

Дякую за відео. Більше контенту :) А як отримати статус readability?

[@AboutProgramming]

Є якійсь процес, де ти маєш підтвердити знання гайдлайнів й правильних практик для певного технологічного стеку, але я поки ніяк не займусь цим)

[@MS-fu3zf]

Дякую за поради і класний контент. Може порадите якісь книги чи ресурси по візуалізації архітектури, типу C4?

[@AboutProgramming]

Я зазвичай або С4 користую, або UML, або просто малюю вже своє щось. Відносно візуалізації, то тут скоріше головне питання в тому, схему чого ми візуалізуємо, а там вже можна підібрати інструмент/стандарт для візуалізаціх. С4 мені дуже подобається, найчастіше малюю container diagram. Якщо про UML, то найчастіше Use Case Diagram та Sequence Diagram. А під все інше все просто квадратики й стрілочки :)

[@kbodka]

Якщо когось цікавить більше деталей про те як працюють процеси в гуглі, раджу прочитати software engineering at google. Дуже класна книжка, свого часу почерпнув для себе багато цікавих речей.

[@AboutProgramming]

Окрім того вона в безкоштовному доступі

[@bogdannaumenko7571]

Дуже цікаво було б послухати про Check-list на прикладі UI... більш детальніше..

[@AboutProgramming]

Важливе питання, тому затягнув з відповіддю. Тільки відповів про чек-лісти в іншому коментарі. Але я тут подумав, можливо варто зробити якийсь спільний комʼюніті чекліст разом з підписниками каналу. Я би накидав базу, а потім зібрати ще фідбек від писників й зробити спільні документи.. Як ідея? :)

[@vasylnahuliak]

Як щодо розбиття merge/pull requests на менші? наприклад створюється окрема гілка в яку попадають такі маленькі МР а потім закидується сквошом в основну гілку

[@AboutProgramming]

Будь-яка довгоживуча гілка створює проблеми, навіть якщо в неї регулярно тягнуться дані за мастера. Чим менші реквести й частіше, тим краще. В Google взагалі все живе тільки в основній гілчці й мерджитися можливо тільки в основну гілку. В WebbyLab ми використовуємо переважно "Git One Flow branching model" - такий собі варіант trunk based development

[@const_g]

Майже нічого нового. Але все одно дякую)) цікаво що за муз обладнання у вас там? Ви музикант ще? Може розкажете трохи про це теж?)

[@AboutProgramming]

Це chromakey)

[@const_g]

​@@AboutProgrammingлол)) а виглядає як справжнє)

[@mikemakhovyk6941]

Привіт! Ти в інтерв’ю казав, що працюєш над фічами, які можуть розроблятися дуже довго. Як в гуглі відбувається цей процес, коли створюються МРи, де ще не весь функціонал реалізовано, десь work in progress, десь можливо якісь заглушки? Чи залишаються явні коменти/ToDo? Чи МР має бути максимально фіналізований, наскільки можливо?

[@AboutProgramming]

Все живе в одній гілці й фічі сховані за feature флагами

[@eolit1o]

Є якісь обмеження у кількості строк коду на один pr?

[@AboutProgramming]

Технічного обмеження немає, є просто рекомендація, що менше зазвичай краще

[@maxbabych52]

На скільки ревьювер повинен бути поглинутий в таск щоб вирішувати чи ця задача вірно зроблена і чи взагалі повинен ревьювер поринати в задачу? Дякую

[@AboutProgramming]

Для readability review це не обов'язково. А от для основного рев'ю дуже корисно розуміти контекст задачі. Й це має бути відповідальність інженера забезпечити рев'ювера контекстом - реквест має бути присвяченій одній зміні, детально описати ідею зміни в реквесті, додати скріншоти, додати посилання на опис задачі, обрати рев'ювера, який вже розуміє задачу чи згоден у неї зануритися

[@maxbabych52]

@@AboutProgramming мені завжди чомусь здавалося що це моя відповідальність розібратися в ревью і виходило що я читаю таск , придумую рішення а потім ще розбираюся як воно реалізовано в пулі , а виходить що було достатньо щоб інженер правильно доніс свою реалізацію. Дуже дякую за слушну пораду і з Новим Роком)

[@AboutProgramming]

@@maxbabych52 так, й якщо щось не зрозуміло, тоді рев'ювер буде задавати питання в коментах й просити пояснити ідею й це буде збільшувати час рев'ю й інженеру доведеться декілька разів повертатися до коду й проходити додаткові ітерації. Через пару таких рев'ю вже сам почне додавати більше контексту, якщо хоче якісне рев'ю

[@logcom482]

В пітоні з стильом написання коду все простіше не зробив відступи і ніфіга не запуститься)))На С хоч пиши весь код в одному рядку)

[@AboutProgramming]

Ну там відступи, а там дужки фігурні)

[@thankfulperson3309]

Як заохотити всю команду до процесу код рев'ю, коли тільки частина більш-менш активна в цьому питанні?

[@AboutProgramming]

Команда має бачити цінність в код рев'ю. Й тут або вони не розуміють цінність й це треба донести (наприклад, що станеться, якщо всі на проекті відмовляться в код рев'ю), або цінності немає - наприклад, якість коду для керівництва не є важливою й тут можливо треба донести спочатку цінність до керівництва.

[@v.ilchenko]

Ми недавно впровадили стайлгайди (до цього тільки лінтер був) і це прямо суттєво спростило рев‘ю. Більшість коментарів зараз - лінки на док. Тут є питання - чи варто в стайлгайд виносити речі, які покриває статичний аналіз? Умовно «використлвуй let/const замість var» Додав би що корисно на проекті налаштувати codeowners, щоб відповідні люди/команди автоматично асайнились. По архітектурному ревю - як це може виглядати в продукті? Коли у нас одна команда працює над одним проектом. Зараз є декілька senior людей, які можуть зарефакторити проект, але це працює поки для команди в ~50 розробників. Коли вас стає 500, по ідеї потрібна окрема команда під це? Гугл гайди рекомендують робити код рев‘ю протягом 1 дня. Як у вас цього дотримуються? Просто цікаво :)

[@AboutProgramming]

Чим більше стайлгайдів вийде замінити статичним аналізом, тим краще. Тоді вже посилання на доки може давати сам лінтер й рев'юверу не треба витрачати час. Відносно codeowners, то це теж є, але чисто для регулювання прав доступу (тобто галочка від овнера теж завжди потрібна). Відносно архітектурного рев'ю в продукті, то можна роботи рев'ю й командою, яка працює над проектом. Тут головне, щоб це просто було й був менеджмент технічного боргу. Коли над продуктом працює 500 людей, тоді це буде 30-50 команд й тут вже питання як узгодити команди між собою. Різні компанії підходять по різному - хтось за повну автономію, хтось за більше централізацію. Я за децентралізацію, але максимальну стандартизацію й узгодження. Це можна зробити за рахунок: 1. Спільна інфраструктура. Вона зможе прогарантувати схожі процеси й підходи, один й той самий статичний аналіз, один механізм деплойменту й моніторінгу та інше. 2. Спільна документація - рекомендаці по процесам, спільні стайлгайди та інше. 3. Спільна дизайн система й набір компонентів/бібліотек 4. Обмін знаннями (презентації рішень від однієї команди іншим та інше) 5. Різні cross team (company wide) efforts. Часто визначаються, як цілі на рівні директорів й кожна команда забов'язана зарезервувати певний час на іх виконання. Відносно код рев'ю протягом одного дня, то тут просто - бачиш, що вісить на тобі реквест, то почав ранок з цього (або як прийшов з обіду). Якщо у рев'ювера немає часу подивитися, а тобі треба терміново, то можеш попросити напряму або ще додати інших людей для рев'ю у кого є час. Або написати в спільному чаті й спитати, хто може швидко глянути.

[@v.ilchenko]

Дуже інформативно, дякую 🎉

[@feelthefreedom]

є можливість поділитись чек-лістом по фронтенду?

[@AboutProgramming]

По фронтенду нажаль чеклістами поділитися не можу, оскільки там частина вимог це приватні вимоги від клієнтів. Але концептуально, це просто документ, який описує стандартні кейси, які мають бути покриті під час розробки фронтенду типу * редірект, коли закінчилася сессія * помилка про проблему з інтернетом, коли пропав звʼязок * лінкі мають відкриватися через CTRL+click й так далі Тут головна ідея в тому, що такі проблеми повторються від проекту до проекту й має сенс десь це документувати, щоб не повторювати помилки. Часом це вирішується за рахунок спільної архітектури, часом процесно, а часом інфраструктурно

[@hesher2301]

У вас позаду така музична студія добре напакована) це просто бекграунд?

[@AboutProgramming]

Просто картинка)

[@hesher2301]

@@AboutProgramming жаль

[@AboutProgramming]

@@hesher2301 ех, сам би хотів

[@eolit1o]

На який позиції ви працюєте в Google? І який у вас левел?

[@AboutProgramming]

L5

[@AboutProgramming]

й додав посилання на мій Linkedin в опис під відео

[@SvoyakD]

Послухав це все і страшно уявити кінцеву вартість реалізуємих фіч в гуглі. Осообливо коли в аналіз коду повинні включатися розробники, які не працювали над поточною задачею. Бо для якісного ревью потрібне повне занурення, потрібно знати всю архітектуру проекта\модуля, інакше такий аналіз будет дуже поверхневий і неякісний.

[@AboutProgramming]

Сам жахаюся, бо дуже дуже дорого виходить. Але вартість помилки ще вища

[@suhovN]

питання щодо review від людини яка має readability status. Хто ассайнить цю людину, автоматизація чи власник ПРу вручну? Чи є явний дозвіл або заборона на те щоб запросити review у декількох таких людей одночасно. Якщо вручну та така людина в іншій команді, як знати що ця людина взагалі доступна, можливо вона out of office у найближчий тиждень або у неї черга подивитись review на стільки велика що твій ПР подивиться тільки через тиждень?

[@AboutProgramming]

Може бути й вручну й автоматично. Якщо автоматично, то всі перевірки пройдуть автоматично. Якщо вручну, то перевірки можна зробити вручну (написати людині, глянути чи на роботі й тд). Про заборону в декілька людей не чув.

[@LyubomyrSemkiv]

З мого досвіду, 8 з 10 ревю які я роблю є неправильними. Воно ніби вирішує проблему, алк створює якісь інші проблеми різного хврактеру, або просто калічно - зайві дії. Глибоке коуд рев‘ю, де перевіряється валідність фікса, а не відповідність якимось сумнівним поактикам є найбільш цінним. Я не подивився все відео, але здається здатність робити такі ревю тут називається рідабіліті статус. Це дцже поавильно і розумно, лише я не розумію, чому у вас при тому дають дизайнити і овнити фічу з непідтвердженими навиками дизайну. Так само як більшість програмістів не вміють програмувати, так само вони не вміють дизайнити.

[@AboutProgramming]

Всі починають з того, що не вміють дизайнити й треба починати з чогось, щоб отримати цей скіл. Тому проектування корисно й для джунів, просто треба ім допомогати в цьому й задачі мають бути відповідного рівня. Відносно рідабіліті це в першу чергу на відповідність стандартним практикам конкретного стеку, проекту й тут не треба глибокого контексту по фічі, але це теж важливо. Тому рідабіліті апрув завжди має бути до стандартного ревʼю від когось з команди (але це може бути одна й та сами людина, яка й рідабіліті апрувить)

[@LyubomyrSemkiv]

​@@AboutProgrammingага, тоді це інше. Це не ability to read code а апрув що code is readable. А дизайнити вичтися краще працюючи "підймайстром" в того, хто вміє дизайнити. Це такий класичний ремеслярський підхід.

[@vasylnahuliak]

Цілий рік записував відео і на кінець вирішив залити? 😂

[@AboutProgramming]

Аби) Завтра вранці планую зняти ще 2 з готеля))

[@mrart5498]

Гарний початок! З нетерпінням чекаємо!