Вхід і реєстрація
Розробка · 19 квітня, 22:47 6417
Oleksandr Shestopal, Ruby on Rails developer. Looking for great remote team to join as Senior/Lead developer

Токсичний код рев’ю

TelegramУсі статті, обговорення, новини для початківців — в одному місці. Підписуйтеся на телеграм-канал!

Є колега, джуніор, хочк 3 роки на проекті, сам з Гон-Конгу. І в нього на все є своя думка. І він вперто її продавлюєю. Апофеоз цьго — код ревью. Комент практично до кожної стрічки. З них може пару багів. Інше це — давай цей метод назвемо так, двай змінну переіменуємо, давай цей тест сюди перенесемо а цей переназвемо. І все аргументується читабельністю коду, або типу просто suggestion. Але додає change-request часто і без нього не заапрувиш + він довше всіх на проекті. Людина працювала вчителем і таке враження що звичка перевіряти домашку лишилась. Я розумію що увага до деталей це добре, але де розумна границя і як її краще пояснити?

Теми: code review, debug, junior, команда
👍ПодобаєтьсяСподобалось0
До обраногоВ обраному0
LinkedIn

Найкращі коментарі пропустити

Karina Kosenko 19.04.2023 22:52

Як так виходить, що джуніор проводить code review? Чи є в команді лід, за яким буде останнє слово (чи це ви і є)?

Перейти до дискусії
Dou Dou 20.04.2023 21:39

Как бы сделал я с высоты более 15 лет в профессии. Все что коллега-джуник тебе накидывает, ты берешь и в трекер описываешь «Переименовывал переменные, методы, улучшал читабельность итд» и отмечаешь время потраченное 1 или 2 часа. Если менеджера устраивает, ну и ок, а если нет, то да начнется экшен.

Перейти до дискусії
Дозволені теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter
Дозволені теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter
John Appleseed 23.04.2023 19:47

Погляньте на їбане.айті , от де ні, а там токсичність код-рев‘ю — в повний зріст.

Відповісти
Підтримати
Global Engineer 22.04.2023 21:33

Зазвичай краще пояснити свою позицію і донести, що так краще (якщо так краще)

Відповісти
Підтримати
Просто Анон 22.04.2023 21:26
джуніор, хочк 3 роки на проекті

ІМХО, людина(толкова) за 3 роки стає експертом на проекті, включаючи розуміння бізнес логіки, як «правильно» писати код для цього проекту і т.п. тому не називай його джуном, хіба що ти там стажор:)
Це нормально що старожил «прищепляє» культуру проекту. Швидше за все твій код гірший бо в іншому випадку на коменти які «погрішують» код ти б так і відповідав, що це буде гріше тому і тому.

все аргументується читабельністю коду, або типу просто suggestion. Але додає change-request часто і без нього не заапрувиш

Якщо він добавляє change-request то це не просто suggestion.

Комент практично до кожної стрічки. З них може пару багів.

Якщо в твоєму коді баги можна знайти під час код ревю то це поганий код.

Якщо це тільки до твоїх пулреквестів то зідзвонись з ним і запитай чому він іншим такі коменти не пише.
Якщо це він всім так пише то він відчуває (а може і є) «гейт кіпером» коду і слідкує щоб туди не пушали сміття. Тут хіба перейняти його культуру написання коду

Відповісти
Підтримати
Oleksandr Shestopal Ruby on Rails developer. Looking for great remote team to join as Senior/Lead developer 21.04.2023 15:46

Вирішив надалі так і робити, максимально грануляції PR

Відповісти
Підтримати
Oleksandr Shestopal Ruby on Rails developer. Looking for great remote team to join as Senior/Lead developer 21.04.2023 15:44

Ми це питання вирішили через введення стандартів. Наприклад до коментівдодаємо анотації blocking, suggestion, neatpicking, question, бо не native speaker, то не завжди ясно що маэться на увазі. Для імен ввели naming conventions з ruby style guide, для specs/tests також і діло пішло на лад. Обговорили все в команді, та й на рівень компанії підняли.

Відповісти
Підтримати
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 07:59
нотації blocking, suggestion, neatpicking, question, бо не native speaker, то не завжди ясно що маэться на увазі.

Можете розшифрувати смисл кожного з названих розділів?

Відповісти
Підтримати
Oleksandr Shestopal
Вільний Художник 21.04.2023 12:11
джуніор, хочк 3 роки на проекті

Звучить як незаймана повія

Відповісти
Підтримати
Dmytro 21.04.2023 11:45

А в чому проблема? Вам людина підкидує інфу як покращити свої навички. Якщо вона має рацію по коду — покращити код, не має рацію — покращити комунікацію й навчитись продавлювати своє бачення.

Ми ж тут тільки одну сторону чуємо. А я зустрічав декілька разів, коли людина жалілась що до її коду до**буються, а насправді там реально код таке лайно, що просто закриваєш очі, бо немає часу з нуля переписувати й треба хоч до якись стандартів довести, хоча б просто імена нормальні дати змінним. Й там буквально до кожного методу «якого хріна він робе це а називається зовсім по іншому», до кожної строчки «якого хріна робота з БД в контролері?».
А якщо писав (недо)сіньор, то «будь ласка прибери усі свої с*ка фабрики абстрактних сінглтонів й зайди нормально» (що характерно для таких людей — вони обожнюють патерни, усюді про них говорять й пхають, але в них не розбираються, назвивають стейти стратегіями, апкаст адаптерами й так далі).

В будь якому разі, повина бути командна ретроспектива, на якій ви б осудили на що витрачається час та чи доцільно це чи ні.

Відповісти
Підтримати
Serge Loboda Senior Software Developer в Access Softek, Inc. 21.04.2023 11:01

Проблема дійсно подекуди виникає.
Коли займаєшся складними речами, часто тій самої мети можна досягнути у різний спосіб. При цьому альтернативні способи не мають суттєвих переваг один над одним. Щоб такі речи не перетворювалися на смаківщину окремих розробників, існують код стандарти та гайдлайни. Але всі кейси до код стандартів не пропишеш, не кажучи про різний спосіб вирішення задач.
Особисто я намагаюся оцінити чи варта переробка (час розробника, час ревьювера, час на роботу пайплайнів, затримка делівері) отриманій від неї користі. Якщо не варта, то або ігнорую, або просто пишу комент у стилі «а от можна було б зробити так». Інколи люди прислухаються та переробляють добровільно.
Але є люди у яких «є моя думка, решта неправильні». Інколи простіше переробити як воно каже і не витрачати час та нерви. Але у клінічних випадках — ескалація на менеджмент. З аргументацією як раз що до співвідношення вартості переробки до користі від неї. Не пам’ятаю щоб у моєму досвіді це заходило у глухий кут і не вирішувалося. Бо це свідчило б про те, що у команді дві, кхм, сумнівних людини, а то може й три (я ж теж не святий).

Відповісти
Підтримати
Антон Кучеренко 21.04.2023 04:38

По-перше:
3 роки джун? Об’єктивно для не інженерних професій джун 1000 годин, мідл 5000 тисяч годин, сініор — 10000 годин (інженерів тут я не беру) я накидав свої 10000 за 2.5 рочки (це не здоровий приклад, я просто фанат і знаю ще це не в плюс моєму здоров’ю, але були обставини та необхідність) але навіть, якщо він не фанат і сидів цілодобово на жопці рівно ± 6к годин він відпрацював за ці 3 рочки (в тексті доречі не уточнювалося, що це перший проект людини)
Тут маленька ремарка:
до інженерних ланок треба дорости, при чому далеко не всім воно треба більшості комфортно бути вмілим але роботягою за сенйорський станком аніж грати в оті ваші ризики, рішення і відповідальність.

По-друге:
Нуууу чоловіче я бачив історію коли людині писали 14 коментарів на 9 строчок коду виправдано, прям з посиланнями на доку, код конвеншени і т.д. при чому без зла, просто з благими намірами;

По-третє:
Зараз ти всім розказав, що такий двіж в твоїй компанії за норму, але навіть якщо ти вирішив звільнятися(звільнився), все одно не дуже гарно так робити і публічно стати людям, з якими в тебе договірні відносини були/є за шиворот не дуже гарно. Прикинь вони б за тобою ходили і так робили, такі уродські компанії і правда є, але ж краще бути людиною і не чинити погано;

По-четверте:
Є стара байка що якщо хочеш швидко перформнути код, виклади його шмат в паблік, скажи що ти самий офігенний і код твій Бест оф зе Бест і тобі швидко пояснять чому тобі не варто було починати програмувати))))))))) жарт звичайно, але б не завадило пару прикладів, строчка і комент, щоб мати змогу чи поспівчувати, чи посміятися, бо так не дуже зрозуміло, яка зі стратегій правильна ))))

По-п’яте:
ти ж розумієш, що кожен наступний менеджер, який візьме тебе в свою команду спершу буде рахувати та прикидувати через скільки часу він стане «токсичним китайцем?»

По-п’яте:
Слухай, а чого ти власне кажучи, ну звичайно окрім срачу, добивався цією статтею? Чи він самоціль?

Відповісти
Підтримати
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 08:10
3 роки джун? Об’єктивно для не інженерних професій джун 1000 годин, мідл 5000 тисяч годин,

1) Мідл таки 5000 годин? ;)
2) Нехай 8 годин робочий день (не вірю, реально треба рахувати 4-6, якщо людина не палає ентузіазмом). Робочий рік 365*5/7*13/14-7 == 235 днів (5/7 бо тиждень, 13/14 бо на відпустку щось йде, −7 на свята), може, даже 230 днів. Врахуємо 235*8 == 1880 годин на рік. Тоді: півроку — перехід від інтерна до джуна. 1880*3 == 5640 — тільки-тільки може вийшов на мідла. А якщо дійсно рахувати по 6 годин — ще не дійшов.

Я не кажу, що згоден з вашими принципами розрахунку і конкретними цифрами, але навіть за ними нема реальних причин заперечувати.

я накидав свої 10000 за 2.5 рочки

Реально це робота на знос. Не можна на таке розраховувати.
І це ще ви не враховуєте, що деякі процеси не можна прискорити — наприклад, усвідомлення робочого циклу великих проектів на власному досвіді.

ти ж розумієш, що кожен наступний менеджер, який візьме тебе в свою команду спершу буде рахувати та прикидувати через скільки часу він стане «токсичним китайцем?»

«Менеджер»? Чому менеджер?

Відповісти
Підтримати
Антон Кучеренко
Антон Кучеренко 23.04.2023 11:46

Ну так на знос, я ж не кажу що це адекватно, просто так склалися життєві обставини, чи це здорове рішення? — ні, чи шкідливе — так, воно просто вирішувало мої проблеми.
Чи можна прискорити процеси? — та цілком, коли за плечами кілька десятків проектів, є досвід факапів та успіхів в плануванні архітектури, досвід роботи на ентерпрайзі, досвід роботи на 4х проектах протягом 1го дня, з постійним стешем та світчем, такі штуки сильно качають, так що я думаю що можна імпрувати архітектурний скіл з прискоренням часу, щоправда, ціною здоров’я.

Відповісти
Підтримати
Valentin Nechayev
Ангуляров Реактин 20.04.2023 22:53

Попадались такие уроды с синдромом вахтера, в качестве аргументов кидались ссылки на каких-то вкатывальщиков и цитаты выебщиков с твиттера. Один чел был ниже меня ростом на голову, а другой моложе на 10 лет. Обое, наверное самоутверждались таким образом. Если вы на одном уровне в команде часть риквестов можно игнорить или реджектить. Пошарь экран на митапе и покажи все эти требования команде, чтоб все, включая менеджеров, опытных дядек это увидели. Хуже когда это твой лид занимается таким, были и такие, что-то делать бесполезно, к счастью ушел с проекта, а другой уволился. Это надо репортить деливери, т.к. он может даже не знать что вы там что-то ревьювите и сколько на это тратите времени и доступа к репе и пулл реквестам у него может не быть

Відповісти
Підтримати
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 08:14

О, приветствую профессионального создавателя технических долгов. ;)

Відповісти
Підтримати
Ангуляров Реактин
Ангуляров Реактин 23.04.2023 17:27

технический долг появляется и накапливается всегда в любом проекте. Если ума у манагера нет закладывать время на его фиксинг, то он растет еще быстрее

Відповісти
Підтримати
Valentin Nechayev
Просто Анон 23.04.2023 17:58
Если ума у манагера нет закладывать время на его фиксинг, то он растет еще быстрее

В менеджера може бути розум поставити прискіпливого і відповідального ліда щоб більшість проблем фіксати ще до мерджа.

От тільки говнокодерам він буде виглядати як

уроды с синдромом вахтера
Відповісти
Підтримати
Ангуляров Реактин
Ангуляров Реактин 24.04.2023 13:00

Потом я смотрел творения этого «ревьювера», обычный говнокод слепленый на коленках, куча неоптимизированых sql, сделал таску на от*бись и закрыл ее. В итоге оказался обычным галерным попрыгунчиком и свалил на другую галеру

Відповісти
Підтримати
Просто Анон
Dou Dou 20.04.2023 21:39

Как бы сделал я с высоты более 15 лет в профессии. Все что коллега-джуник тебе накидывает, ты берешь и в трекер описываешь «Переименовывал переменные, методы, улучшал читабельность итд» и отмечаешь время потраченное 1 или 2 часа. Если менеджера устраивает, ну и ок, а если нет, то да начнется экшен.

Відповісти
Підтримати
Уставший архитектор 21.04.2023 00:58

«А почему ты не можешь сразу писать код нормальный?»
«Ты вроде синьйор, а за тобой джуны поправляют»

Відповісти
Підтримати
Dou Dou
Dou Dou 22.04.2023 13:04

Если именно в таких формулировках, то делаю ахуе.вшее ебало и говорю «разговор окончен» и иду заниматься своими делами.

Відповісти
Підтримати
Уставший архитектор
Уставший архитектор 22.04.2023 16:19

«Мы решили не платить за последний квартал, качество кода не соотв. пункту договора о качестве кода»

Відповісти
Підтримати
Dou Dou
Dou Dou 22.04.2023 20:44

Ну мне фиолетово заплатили ли конторе за квартал или нет.

Відповісти
Підтримати
Уставший архитектор
Уставший архитектор 23.04.2023 12:36

Если сам себе контора, то не фиолетово.
Если гребец на галере, то два таких случая — и «вільна каса»

Відповісти
Підтримати
Dou Dou
Dou Dou 23.04.2023 13:51
Если сам себе контора, то не фиолетово.

Конечно. Это вообще другая реальность по другим правилам. А еще будте добры формализованные пункты договора о качестве кода. Еще, насколько я знаю, во всем мире никто не знает, что такое качественный код.

Если гребец на галере, то два таких случая — и «вільна каса»

Если проект не магазин кошачьего корма на ноддджс, то ввод нового гребца в проект, чтобы он хоть как-то работал это полгода.

Відповісти
Підтримати
Уставший архитектор
Уставший архитектор 23.04.2023 15:45
Если проект не магазин кошачьего корма на ноддджс, то ввод нового гребца в проект, чтобы он хоть как-то работал это полгода.

Ахахаха, неделя максимум. Сейчас не 2021.

Відповісти
Підтримати
Dou Dou
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 16:02
Ахахаха, неделя максимум.

Точно магазин даже не кошачьего корма, а семечек.

Сейчас не 2021.

Ничего существенно не поменялось.

Відповісти
Підтримати
Уставший архитектор
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 18:18

(ушёл плакать о несправедливости мира)

Відповісти
Підтримати
Уставший архитектор
Oleksandr Suvorov https://spgr.org.ua/uk/ в Foundries.IO 20.04.2023 13:36

Якщо у вас є якісь спільні мітинги щодо робочих процесів, це виноситься (без персоналій) на них: «а давайте обговоримо наш процес код-ревью, як це робити, як покращити процес, як уникнути зайвих зауважень, які сповільнюють розробку продукту»... ;-)

Відповісти
Підтримати
Anatoliy Rentgenovich 20.04.2023 11:55

странная ситуация
3 года и джун
код не показал, может ты там венгерскими нотациями все пишешь

Відповісти
Підтримати
Dou Dou 20.04.2023 14:37
может ты там венгерскими нотациями все пишешь

Можно и венгерскими, если весь проект написан венгерскими.

Відповісти
Підтримати
Anatoliy Rentgenovich
Олексій Хількевич 20.04.2023 11:36

в принципі це не технічна проблема, а людська

Ти не написав скільки ти на проекті

І впевнись сам спершу що ти не перебільшуєш суб’єктивно

Якщо це разок то move on, якщо реально часто, і він там вже три роки, то розрулити її топорно ябеднічая чи упираясь я б не рекомендував, навіть на ванонван, якщо менеджер не твій кореш і сам не в курсі що китаєць такий

отримаєш атветочку через місяць сам

Підніми питання з менеджером, але не топорно ябеднічая, а прощупай що він скаже і чи він налаштований взагалі щось з тим робити

якщо тіма велика я б перед тим заручився думкою пари інших, шанс великий що всі так вважають, разом і батька легше бити

Якщо тіма маленька, і він там довше всіх, то манагер там слабкий, то сорян шансів в тебе реалістично нема швидко розрулити якось позитивно, а шансів нагребти проблем море, move on, терпи і шукай краще як є де можливість.

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

Є тактики і почорніше, але то вже залишу за темою

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

Впевненість що ти можеш розрулити ситуацію виключно аргументами і логікою — явна ознака що ти до такого бою не готовий.

Жодна компанія не варта того щоб менеджити менеджера, з досвіду кажу

Відповісти
Підтримати
svikso 20.04.2023 12:45
Якщо таки налаштований розрулити це зі слабким менеджером, не рекомендую відповідати тим самим підходом до рев’ю китайцю прямо, скоріше підготуй манагера, це може зайняти значний час, вказуй частіше який китаєць некваліфікований, і може місяців через 6 китаєць налажає по крупному і ти зможеш без ризику на якомусь ванонван сказати що він козел і це тягнеться давно і ти просто не хотівцю проблему піднімати ітд

Нічо се. Це якось дуже підступно, ні? За спиною менеджеру повільно лити отруту у вуха і чекати поки колега налажає. Я б вже краще працював у команді з таким код рев’ю за.обою, чим з людиною яка втиху тебе зливає.
Імхо, найкращий варіант це dou.ua/...​rums/topic/43120/#2611243
Під кожним коментом хай тикає у відповідний пункт код-стайл доки.
Не має такого пункту або код-стайл доки? -> нема змін в PR.
Хай тоді колега джун пише кодстайл доку, затверджує її зі всіма і всім буде профіт. Можна ще буде йому відтячити під час затвердження кожного пункту кодстайл доки.

Відповісти
Підтримати
Олексій Хількевич
Олексій Хількевич 20.04.2023 13:00
якось дуже підступно

С волками жить по волчьи выть

Не люблю зміюк, але якщо трапляються з ними треба вміти вести себе відповідно.

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

Відповісти
Підтримати
svikso
Victor Gubin Programmer 20.04.2023 16:57

По тим кого вважають за колег таких топіків не створюють. OP прямо написав, що вважає його за конкурента і волів би від нього здихатись. Судячи з усього це на взайм. Якщо дивитись на справу зі сторони — воно саме так і є, це пряма економічна конкуренція як вона є. Цікаво, що ніхто не рекомендував чесні методи конкуренції — наприклад надавати кращі послуги, тому самому менеджеру.

Відповісти
Підтримати
svikso
Олексій Хількевич 20.04.2023 19:50
ніхто не рекомендував чесні методи конкуренції

в нечесній грі програє той хто веде чесну гру селяві

І тут нижче ціла простиня коментарів чесних методів :)

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

Відповісти
Підтримати
Victor Gubin
Уставший архитектор 20.04.2023 17:02

ППКС

Є тактики і почорніше, але то вже залишу за темою

Ну позязя, мы очень просим!

Відповісти
Підтримати
Олексій Хількевич
Олексій Хількевич 20.04.2023 18:02

нуууу, тут на книжку можна нашкрябать, може нашкрябаю колись

тут багато спекуляцій зразу кажу, без бути в тімі важко вгадувати

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

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

тут спекуляції закінчились

але важливо що формально він джун і не тімлід

подібні чувіки часто люблять мати ще якусь свою власну область в проекті, захищати її як свій город. Там модуль якийсь, фіча якась в якій вони «експерти». Сімптом — таска приходить, чувік хватається проактивно і не дає нікому з тим колупатись. А на інші таски не так активно реагує, можливо навіть спихує з себе, типу он вася новенький може взятися для розвитку.

Ідентифікуєш цю область, фічу. Неважко, але може зайняти час, рецепта нема, скоріше слідкувати за реакціями треба. Готуєш зі свого боку пропозицію по імпрувменту чи нєєбіческому рефакторінгу чи заздалегідь готуєш ідею в цій області. Без його відома кидаєш в діскашен. Чувак звісно клює. Ти кажеш типу давай я буду ревью робити. Він на крючку. Розтягуєш процес на місяць, даючи публічні поради і часом кажучи що все якось довго робиться.

І топиш його вже ти

Підбираєш в цей час таски поменше і попростіше, особливо в нього, щоб у нього було менше причин казати що він працює над яимось іншим, перформиш ці таски «краще» нього. Можна навіть з’їдати його час дзвнками й питати поради цілу годину почастіше і дякувати публічно за це але дякувати без ентузіазму. Через місяць на ван он ван тупому менеджеру кажеш, що китаєць не перформить але в тебе є ідея (будьяка з умної книжки) як йому допомогти розвиватися

Неформальний лідер вже ти

неетичні фантазії на кофєйной гуще короч, не сприймай серйозно ))

Відповісти
Підтримати
Уставший архитектор
Олексій Хількевич 20.04.2023 18:32

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

www.goodreads.com/...​843.The_Career_Programmer

І ще є класіка 48 laws of power, це просто мастхев

в принципі це все вимагає якогось внутрішнього виклику щоб переступити через якісь власні етичні внутрішні норми, які у всіх є, і більшість людей не розіб’ють свій моральний компас заради якихось п***юків і це добре. Але бути aware що читає менеджер чи сусід не завадить нікому

Відповісти
Підтримати
Уставший архитектор
Alexander Gatsenko 20.04.2023 11:33

Во первых, смысл код-ревью джуниора заключается в том, что бы джуниор поднял свой уровень, поэтому как-то удивляет, что джуниор делает подобное ревью. Во вторых, любые замечания по стилю кода должны приниматься только после добавления в проект документа «Code Style», если нет документа или замечание по стилю (включая имена) ни как не связано с документом, то можно (а по мне, так НУЖНО) мягко послать ревьювера на создание/расширение документа. Ну и по всем замечаниям надо задавать вопросы — как это улучшит читабельность, производительность и т.п. Вполне возможно, что он не джуниор и замечания имеют место быть, но без подробных пояснений вы не поймете, зачем все эти изменения.

Відповісти
Підтримати
Dmytro Lapshyn Tech Lead в Fulcrum Rocks 21.04.2023 11:25

Очень здравый комментарий, поддерживаю! Вместе с тем, я не очень себе представляю, как в такой документ «ужать» половину (а то и больше) содержания книги Code Complete?

Если кто не читал — там не про отступы и переносы строк (хотя, отчасти, и эта тема затрагивается в плане общей читабельности кода), а о том, как писать код так, чтобы он читался как книга.

Відповісти
Підтримати
Alexander Gatsenko
Володимир Корнійчук Senior Software Engineer (C++) в Infopulse 21.04.2023 13:01

Та можна прямо і всю книгу в той документ зкопіпастити. І з цього моменту то вже буде відповідальність рев’ювера знайти потрібний пункт і дати на нього посилання.

Відповісти
Підтримати
Dmytro Lapshyn
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 10:28
Во первых, смысл код-ревью джуниора заключается в том, что бы джуниор поднял свой уровень, поэтому как-то удивляет, что джуниор делает подобное ревью.

Это если предполагать, что джун — джун во всём. Возможно, в данном случае и так, но в общем — никто никогда не должен предполагать, что другой слабее его во всём (даже если этому другому 2 года и он ещё в яслях).

Ну и косячить может каждый. Поэтому кросс-ревью даже от джунов полезно.

о первых, смысл код-ревью джуниора заключается в том, что бы джуниор поднял свой уровень, поэтому как-то удивляет, что джуниор делает подобное ревью.

Ага.
Когда я читал рассылки freebsd, самые интересные обсуждения шли обычно в src-all, которая была аналогом таких ревью :))

Відповісти
Підтримати
Alexander Gatsenko
Oleksii Ulanovych 20.04.2023 09:39

знакомо. на прошлом пректе самый частый комент был добавь/удали перенос строки. когда написал в общий чат что не тратьте свое и чужое время, а вместо этого добавьте правило в линтер, обозвали токсичным :)

Відповісти
Підтримати
Pavel 20.04.2023 13:50

И правильно обозвали. Ведь можно было написать что-то вроде «ребята, у меня есть суперовая идея, как осчастливить и вас, и себя, и сделать мир лучше — надо добавить правило в линтер! Готов создать таску, и даже поработать над ней в ущерб основному заданию»

Відповісти
Підтримати
Oleksii Ulanovych
Bot Bot 21.04.2023 00:03

Ну тоесть заебывать мегаздротскими переносами это ок, а сказать что тратиться время и это факт это токсичность? збс канешн.

Відповісти
Підтримати
Pavel
Володимир Корнійчук Senior Software Engineer (C++) в Infopulse 21.04.2023 13:03
а вместо этого добавьте правило

Так, сказати «у мене тут є проблема, і підіть-то вирішіть її для мене» — це токсичність. Не токсичність — це запропонувати зробити самому, а потім і зробити.

Відповісти
Підтримати
Bot Bot
Bot Bot 22.04.2023 14:31

Добавити самому в лінтер правило де такі задроти? :-) Це буде іще більше срачу :-)
Я колись був на такому проєкті, я згодом просто пішов звідти, я двичайно за стандарти коду щоб легше читалось але коли все код ревью і отаке задроство то пробіл то таб, чи то десь грамматична помилка в назві змінної всередині методу...нехай самі в своєму клубі задротів працюють :-)

Відповісти
Підтримати
Володимир Корнійчук
Антон Кучеренко 22.04.2023 15:19

Насправді, коли є такі складності, то лінтер і претіер просто мастхев, там всього тих правил штук 20-30 сіли з командою проклацпли, зробили документ для планів і пішли радіти життю, навіть на ентерпрайзів то до тижня роботи, навіть з автозміною усього, усього прям усього коду проекту з новими правилами, класно звичайно коли він є спочатку, але такі штуки допомагають, все одно у всіх хто приходить налаштування іде зі старого проекту, тому це краще виправити або лінтером прям інтегрованим в вебпак і проект на нативному рівні з препуш лінтами, чи препуш пайпами, просто одразу питаєш, чувак що ти юзаеш? вс, джет Брейн, атом — ось тобі плагін, ось інструкція з налаштування і все😉

Відповісти
Підтримати
Bot Bot
Bot Bot 22.04.2023 23:59

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

Відповісти
Підтримати
Антон Кучеренко
Антон Кучеренко 23.04.2023 00:52

Окей, давай приведу кілька прикладів, де комент про перейменування функції доцільний:
1. Bad:

function isBetween(a1: number, a2: number, a3: number): boolean {
return a2 <= a1 && a1 <= a3;
}
Good:

function isBetween(value: number, left: number, right: number): boolean {
return left <= value && value <= right;
}

2. Bad:

class Subs {
public ccId: number;
public billingAddrId: number;
public shippingAddrId: number;
}
Good:

class Subscription {
public creditCardId: number;
public billingAddressId: number;
public shippingAddressId: number;
}

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

Відповісти
Підтримати
Bot Bot
Serg Vasylchenko 23.04.2023 09:01

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

Відповісти
Підтримати
Антон Кучеренко
Антон Кучеренко 23.04.2023 11:33

Можливо, але як на мене твоя теза вірна до перших 50 бізнес сутностей, які постійно щотижня міняють логіку парсингу, тип запитів, ступінь доступності по правам користувача, так щоб в списку з 40 оголошених змінних чи об’єктів на 20-40 полів ти точно був впевнений, що міняєш, без запуску коду. Бо інакше серед десятку ід можна пару днів шукати. (юзеру, його пк, версії встановленої аплікухи, версій іншого софта, версій сутностей в залежності від прав і т.д. і т.п.). Коли тобі 20 ід прилітає дуже легко стрельнути собі в ногу

Відповісти
Підтримати
Serg Vasylchenko
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 07:51
двичайно за стандарти коду щоб легше читалось але коли все код ревью і отаке задроство то пробіл то таб

Ви за стандарти але проти стандартів. Cool.

Добавити самому в лінтер правило де такі задроти? :-) Це буде іще більше срачу :-)

Тому що нормально це автоматизують.
Є тулза, є файл конфігурації. Викликай перед комітом. Можеш автоматом, можеш вручну.

Відповісти
Підтримати
Bot Bot
Уставший архитектор 23.04.2023 12:38
Є тулза, є файл конфігурації. Викликай перед комітом. Можеш автоматом, можеш вручну.

А интегрировать такое в тузлу для кодревью — вообще идеально

Відповісти
Підтримати
Valentin Nechayev
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 16:03
А интегрировать такое в тузлу для кодревью — вообще идеально

Ну зарежектит она PR, это не поможет его автору, если он не обладает такой же настроенной тулзой.

Відповісти
Підтримати
Уставший архитектор
Уставший архитектор 23.04.2023 18:04
он не обладает такой же настроенной тулзой.

Почему?

Відповісти
Підтримати
Valentin Nechayev
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 18:18

А чем бы это помогло?

Відповісти
Підтримати
Уставший архитектор
Уставший архитектор 23.04.2023 20:24

Задетектить и исправить проблемы до коммита?

Відповісти
Підтримати
Valentin Nechayev
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 20:29

То есть «тулза для кодревью» должна запускаться локально?

Відповісти
Підтримати
Уставший архитектор
Oleg Artyomov Java Tech Lead в Intapp 23.04.2023 18:49

Ну сделай код стайл чек частью билда. В гитлаб например можно настроить, чтобы на каждый МР запускался билд.

Відповісти
Підтримати
Valentin Nechayev
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 19:53
Ну сделай код стайл чек частью билда.

Спасибо, кэп, так и делали. Но при чём тут это к моему комментарию?

Відповісти
Підтримати
Oleg Artyomov
Антон Кучеренко 23.04.2023 12:01
то пробіл то таб, чи то десь грамматична помилка в назві змінної всередині методу

Це тобі ввічливо натякали, підвчити іде, хоча б на рівні базових налаштувань. Насправді то були дуже вічливі і абсолютно милі люди. Я хотів би бачити твою істерику якби тобі в евалюєйшені написали: «невпевнений користувач пк (використовує марковану клавіатуру та мишу), швидкість лінійного набору менше 500 символів на хвилину». Ти б напевно собі в істериці навіть на дупі волосся б повисмикував. Я ржу чувак, просто ржу, якщо що, то в усіх іде є налаштування прекоміту і плагіну претіера є, постав на розфокус буде тобі щастя (в людьських, а не блокнотах є прям в самій іде).
Чому усі такі агресивні, не зрозуміло 🤷

Відповісти
Підтримати
Bot Bot
Bot Bot 24.04.2023 17:47

Туши пукан, істерик )))) Автокомпліт та грамматика в назві методу тут ні до чого, чи ти не кодер взагалі а так — мімокрокодил? :-)

Відповісти
Підтримати
Антон Кучеренко
Антон Кучеренко 24.04.2023 18:07

Чого ти такий нервовий, окей якщо я не розумію то поясни мені в чому саме полягає профіт від написання нарочито безграмотних слів, чи форматування коду «по особливому» не так як у всьому проекті, типу от по своєму по специфічному, от якщо без жартів, то на минулих проектах ми зазвичай навіть про таку фігню як порядок інкапсцляції css класів домовлялися, просто один раз домовилися налаштували лінтер і все, просто для того, щоб не витрачати зайві 10-30 сек коли треба щось знайти, бо коли код очікуваний, то ти відкрив глянув оком одразу туди куди треба і вирішив проблему. Якщо ти незгоден, то не переходь на особистості просто поясни в чому профіт від «спагетті коду» чи «не таких як у всієї команди, а особливихрішень стилізації» окрім збільшення ціни підтримки

Відповісти
Підтримати
Bot Bot
Oleksii Ulanovych 22.04.2023 16:04

у автора немає проблем, автор написав гарний код на який не лається лінтер. приходить інша людина з суто власним баченням і хоче щоб автор щось там поміняв, виникає питання, чому саме автор має поступитися і щось там робити, якщо це одне бачення прекрасного проти іншого бачення прекрасного?

Відповісти
Підтримати
Володимир Корнійчук
Oleksii Ulanovych 22.04.2023 16:25

отже виходить, що автор коду за умовчування неправий і має зажди слідувати вказівкам кожного хто вирішив щось там написати в ревью, навіть якщо це суто питання смаку і на даний момент не формалізовано?

Відповісти
Підтримати
Pavel
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 07:55
а вместо этого добавьте правило в линтер, обозвали токсичным :)

Может, и правильно назвали. Ибо: n-t.ru/ri/pr/zp07.htm
читать обязательно, потому что термин bikeshedding давно вошёл в деловой жаргон.
Обсуждая переносы и пробелы против табов, люди отдыхают от загруза мозгов основной задачей, формально находясь по-прежнему в поле рабочих вопросов. А ты собрался лишать их этого отдыха!

Відповісти
Підтримати
Oleksii Ulanovych
Full Stackly 20.04.2023 08:32

Хоча зазвичай це не продуктивно, але в такому разі чому но проводити йому такі самі рев’ю? Це ж не потребує особливих зусиль

Відповісти
Підтримати
De Money 20.04.2023 03:43

Ніхто не знає, як краще у вашому випадку. Є шанс, що він там реально по темі робить зауваження, а ви там бозна як іменуєте і пхаєте ті тести і він молодець)

Для початку — зауваження мають бути аргументованими. «readability» та «suggestion» — не аргументи. Чому конкретно не читабельно — прекрасно пояснюється, якщо це реально проблема. І це пояснити — довше, ніж перейменувати або пересунути той тест. І там «не читабельно, бо Х краще, ніж Y» — це теж не аргумент. Аргумент має бути не від особистих вподобань, а від логіки. Ну і таки трішки конвеншен може допомогти.

Відповісти
Підтримати
Sam Kerzhnerman General Manager в DomainOptions, Inc. 20.04.2023 00:08

чего то переименовать — у вас должно быть нейминг конвеншн согласно которому именуются вещи и тогда не будет никаких больше сагжешенов
правила форматирования кода — туда же
это должно решить вопрос
ну и менеджер должен быть или лид который может осадить особо яркую личность

Відповісти
Підтримати
Dou Dou 20.04.2023 00:09
у вас должно быть нейминг конвеншн

Которого не бывает на проекте почти никогда, потому что когда-то в самом начале проекта всем было впадлу, а сейчас уже поздно.

Відповісти
Підтримати
Sam Kerzhnerman
Sam Kerzhnerman General Manager в DomainOptions, Inc. 20.04.2023 00:13

никогда не поздно, вперед и с песней, а кто не гребет со всеми в ногу того мы потеряем
но вообще конечно главное результат, а остальное это мелочи (синтаксис, названия и прочее)
из-за одного человека их вводить глупо
я бы прежде всего смотрел на результат и если кто то мешает его достигать то с ним нужно разобраться
провести воспитательную беседу и пояснить что от него ждут результат а он своей мелочностью снижает перформанс и это не то что мы ожидаем от него

Відповісти
Підтримати
Dou Dou
Victor Gubin Programmer 20.04.2023 00:42

Чё? Лид объявляет — с сегодняшнего дня код стайл такой то, скажем Google. Стоит Sonar если скан не проходит — комит из PR-а в реджект. Этот вопрос решен автоматом. Там правда другие формальные поводы найдутся организовать конфликт, если к нему есть резкие объективные причины — скажем расходжение объективных интересов. Например три года держат в джуниорах, чтобы не платить или наоборот избавится и взять дешевых контакторов на аутстафинг. Естественно любого не идиота должно батхернтнуть что тебя кидают. Однако не всегда есть вывод пойти по собеседованиям, а иногда кризис скажем как в 2008 и хотелось бы все послать нахер и свалить — но временно нельзя.

Відповісти
Підтримати
Dou Dou
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 10:31
чего то переименовать — у вас должно быть нейминг конвеншн согласно которому именуются вещи и тогда не будет никаких больше сагжешенов

Никакая такая конвенция не решит _всех_ вопросов. Хотя действительно может сократить их в 10-50 раз.

Відповісти
Підтримати
Sam Kerzhnerman
Suburban Meme 19.04.2023 23:52

В дискуссиях с бронелобыми обычно помогают только ссылки на рекомендации или статьи известных дядек (не всегда правда) Например с именованием тестов довольно просто все: показываешь такого плана статью learn.microsoft.com/...​it-testing-best-practices и договариваетесь следовать рекомендациям оттуда. А еще лучше записать это в тим агрименты и тыкать носом всех бронелобых.
Мне самому в команду вместо девелоперов внедрили отряд дружных джаваскриптеров (включая тимлида джаваскриптера) с нулевым опытом в .net и у них тоже на все свой взгляд (коллективный) . Аргументы в принципе не действуют. Начинаешь что-то объяснять в ревью джаваскриптер говорит давай у других тиммемберов спросим (тоже джаваскриптеров) и они коллективно пропихивают какую-то дичь
Вот например из последнего что эти проактивные ребята продавили везде в лямбдах давать «осознанные» названия переменных. Например:

List<LongComplexEntityName> entities;
вместо
var firstEntity = enitities.First(x => x.Id == 1);
джаваскриптеры продавили лепить вот такие имена
var firstEntity = enitities.First(longComplexEntityName => longComplexEntityName.Id == 1);
// или с моками вообще бредятина
Mock<MySuperLongNamedService> mySuperLongNamedServiceMock; mySuperLongNamedServiceMock.Setup(mySuperLongNamedService => mySuperLongNamedService.GetAsync(.....

Відповісти
Підтримати
Dou Dou 20.04.2023 00:16
Вот например из последнего что эти проактивные ребята продавили везде в лямбдах давать «осознанные» названия переменных. Например:

Интересно как проактивные ребята будут именовать вторую, третью linq projections

Відповісти
Підтримати
Suburban Meme
schwarzlichtbezirk 20.04.2023 01:33

Действительно, дичь какая-то. Ну можно объяснить, что длинные имена даются глобальным переменным, и тем что юзаются в других модулях. А локальные во всяких замыканиях, циклах, или просто внутри блока — можно короткими именами.

Відповісти
Підтримати
Suburban Meme
Дмитрий Корбаницкий Python Developer 20.04.2023 09:11

Есть такое правило/рекомендация, вроде из «Чистый код» Мартина: «длинна имени переменной должна быть пропорциональна области её видимости».

Відповісти
Підтримати
Suburban Meme
Suburban Meme 20.04.2023 16:05

Мой аргумент был не из «Чистого кода» но смысл был по сути такой же, область видимости этой переменной минимальнее не придумаешь, к тому же если я делаю сетап для мока
mySuperLongNamesServiceMock.Setup(x => x.
то абсолютно очевидно какой тип у Х и давать ей длинное «осмысленное» имя не имеет смысла
Но вот что ответил тимлид джаваскриптер: в нашей предыдущей команде (по всей видимости джаваскриптеров) была такая же дискуссия и мы пришли к выводу что лучше давать длинные, понятные имена

Просто в случае с лямбдами даже «Чистый код» не нужно читать. Достаточно прочитать любую книгу по .net или пару статей на learn.microsoft.com где будет что-то с лямбдами и обнаружить что никто не дает такие называния переменным. Я не знаю что там творится в джаваскрипте но даже если там все еб**лись и дают такие имена лямбда параметрам, это не значет что можно прийти на .net проект и делать тоже самое. Я же не лезу в джаваскрипт с фигурными скобками на отдельной строке или названиями функций с заглавной буквы. А эти броневики ссылаются на то что они там согласовали в своем предыдущем проекте на другой технологии)

Відповісти
Підтримати
Дмитрий Корбаницкий
schwarzlichtbezirk 20.04.2023 23:59

Вообще они зря распинаются, поскольку в js-проектах практика ровно та же самая. Стрелочные функции, которые юзаются как лямбды, пишутся ровно также с короткими переменными аргументами, x => x.Id === 1.

Відповісти
Підтримати
Suburban Meme
Serg Vasylchenko 23.04.2023 09:07

И лучше не переменную делать с ПравильнымПонятнымКрасивымИменем, а сужать контекст, где такая дичь на 30 символов не понадобится.

Відповісти
Підтримати
Дмитрий Корбаницкий
Олег Senior React Developer 20.04.2023 16:17

Когда итерируется массив с имененем множества (entities), то внутри итерации элемент называется так же в единичном числе (entity).
Для всяких сильно элементарных стрелочных операций в map/filter/reduce/find/.. достаточно писать первую букву (e => ...)

Відповісти
Підтримати
Suburban Meme
Suburban Meme 20.04.2023 16:28

Ну я за «стрелочные» операции и говорю (лямбды). Может для джаваскриптеров из моей команды нужно это слово применить и она согласятся)

Відповісти
Підтримати
Олег
Serg Vasylchenko 23.04.2023 09:04

Они только с книжек повылазили, где талдычат что надо делать «правильные длинные» имена и т.д. Переубедить их сможет только личный опыт через пару лет.

Відповісти
Підтримати
Suburban Meme
Bot Bot 19.04.2023 23:14

Е якись лід, чи манагер, чи ще кому можна показати шо за дічь він вимигає? Скинь цей ПР з його вимогами і скажи шо з такими темпами всі естімації тре множити на два, а толку нуль.

Особливо я б такого, після двох попереджень, вигнав нах, ненавиджу уопоротих дартаньянів.

Відповісти
Підтримати
Sam Kerzhnerman General Manager в DomainOptions, Inc. 20.04.2023 00:21

именно так, этот вопрос лежит скорее в области политической чем технической и решить его должен менеджер

Відповісти
Підтримати
Bot Bot
Dou Dou 20.04.2023 00:31

менеджер будет вместо работы разруливать проблемы сопливых детишек, которые поссорились как переменные именовать

Відповісти
Підтримати
Sam Kerzhnerman
Sam Kerzhnerman General Manager в DomainOptions, Inc. 20.04.2023 00:36

ну вообще то да это как раз часть работы менеджера, этому еще и учат специально

Відповісти
Підтримати
Dou Dou
Victor Gubin Programmer 20.04.2023 00:51

Так это его/ее работа блин. Иначе зачем тащить на себе балласт который не программирует, не тестирует и не выясняет требования ? И при том ещё и не решает конфликты, которые неизбежны, в работе у всех есть объективные интересы которые вступают в конфликт. Скажем у тестировщика — интерес найти баг, у программиста интерес чтобы никаких багов не было (хотя такого не бывает и их просто нужно быстро фиксить, если конечно баг это в принципе — баг). БА и UX/UI как и клиенты — всегда хотят все сразу, без ограничений технологий, безплатно и вчера. С менеджером, по Стратоплану — алкашом, который нихера не делает, типа «я с клиентом договорился — что они всем будут управлять» все очень быстро завалиться с огромным треском. Хороший менеджер, напротив на вес золота. У него/ее как правило вообще таких войнушек не бывает. Большинство конечно где то посредине. Впрочем это всех касается, программисты не исключение. Ляп-ляп и в прод или говнокодеров когда вместо кода один баг сплошной или писателей ядер и велосипедов на голом месте с провалом всех сроков, вместе с теми кто без требований делает вообще непонятно что и т.п. тоже хватает. Впрочем все такие состояния — они не вечные, они временные.

Відповісти
Підтримати
Dou Dou
Bot Bot 20.04.2023 02:43

От этого страдает деливери и проседает велосити тимы,а это деньги и причем заметные. К тому же токсичные дартаньяны разваливают команды, найм и сбор тимы по новой это еще большие деньги. И как не раз подметили, его(менджера) работа.

Відповісти
Підтримати
Dou Dou
Ярослав Характерник Golang Developer в DocHQ 19.04.2023 23:00

Без доступу до конкретних прикладів коду то складно визначити хто правий.

Я раніше прискіпливо проводив code review, а коментарі писав з приставкою major чи minor.
Всі major мали бути виправлені, minor-и за бажанням, після цього апрувив.

Також достатньо було пару апрувів від команди з 5-ти розробників, щоб замержили PR, навіть якщо там був major.

Відповісти
Підтримати
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 10:40

Ось чому в Gerrit система оцінок (за замовчуванням):
+2 «згоден, беру на себе відповідальність за результат»
+1 «згоден, але гарантувати не хочу»
-1 «не згоден, але не заперечую остаточно»
-2 «не згоден, радикально проти (і беру відповідальність за порушення строків доставки результату, якщо моя заборона була невірна)»
Для сабміту звичайним автором потрібно мати хоча б один +2 і жодного −2. «Володілець» репи може продавити результат крізь чуже −2, але це теж буде зафіксовано в історії.
При наявности major ставиш −2, minor — −1.
І таких ситуацій як

достатньо було пару апрувів від команди з 5-ти розробників, щоб замержили PR, навіть якщо там був major.

не буває.

А всі ці фуфлові підробки як bitbucket/github/etc. — не дають якісних засобів контролю за результатами ревʼю.

Відповісти
Підтримати
Ярослав Характерник
Anton Nizhegorodov Staff Software Engineer в Audienzz 25.04.2023 21:03

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

gerrit-documentation.storage.googleapis.com/...​tml#_reviewing_the_change

Відповісти
Підтримати
Valentin Nechayev
Олексій Хількевич 25.04.2023 21:58

мушу попередити, що gerrit викликає звикання і вимагає звикання і не всім заходить, ну крім нечаєва :) там дуже гнучка конфігурація і можна розширити ці +/- в будьщо, назвати будь-як і конфігурувати пермішени будь-як, але в цій мощній конфігурації є мощний ньюанс, який зветься мощна мова пролог

короче нра, але якщо чисто на серце без конкретного кейса, коли відомо і свідомо нащо саме герріт, я б в нього якусь команду не кидав, you have been warned

рекомендую завжди таки спершу «фуфлову підробку» gitlab/github/gitea

а такто кльова гіт-репа справді

Відповісти
Підтримати
Anton Nizhegorodov
Valentin Nechayev архімаггриб в Дарницькі печери 27.04.2023 12:30
але в цій мощній конфігурації є мощний ньюанс, який зветься мощна мова пролог

В останніх релізах вже нема, замість цього таблична конфігурація. І якщо дуже хочеться — можна замінити на свій плагін.
Ну і по портовському досвіду проблему переклали на Jenkins, який тупо одразу ставив Verified:+1 якщо у нього не було правил для цеї репи.

і не всім заходить, ну крім нечаєва :)

Якщо це тільки через той (вже видалений) пролог, то забудь, вже неактуально.
Якщо через щось інше, хотілось б подробиць.
Gerrit дозволяє робити і в стилі «криворукі розроби понад розподіленим CVS», але навіть у цьому випадку дає більше зручности.
А якщо нормально робити — то він дає для цього більше засобів аніж будь-який конкурент — я маю на увазі перш за все його систему с patchsetʼами.

Проблема в том, що коли частина користувачів вміють якісно робити, а частина — як з CVS, ця друга частина страждатиме з-за своє невіглацтво:) Оце дійсно проблема, коли «інших людей у мене для вас нема» (tm).

рекомендую завжди таки спершу «фуфлову підробку» gitlab/github/gitea

Вони можуть бути (а можуть і не бути) гарніші у випадку вдубованого (описка, залишу;)) CI, якщо це зручно. У Gerrit такого нема. Це, да, недолік для малих розробок. Але як тільки стає треба дійсно окремого CI — треба якісно перерозмислити.

gerrit викликає звикання

Тим що в ньому дійсно добре;))

і вимагає звикання

Я, навпаки, складно звикав до тих недоробок як BitBucket;))

Відповісти
Підтримати
Олексій Хількевич
Мавпа программиста 19.04.2023 22:54

Додайте гайдлайни, код рев’ю чекліст, та северіті файндінгів (з поясненням де і який левел і які треба фіксити — наприклад тільки 4 та 5 рівень, а 1,2,3 на розсуд автора реквесту).

Це знизить продуктивність закривання пр, але повністю вирішить проблему з цим токсіком.

Відповісти
Підтримати
Karina Kosenko PHP developer в IT Craft 19.04.2023 22:52

Як так виходить, що джуніор проводить code review? Чи є в команді лід, за яким буде останнє слово (чи це ви і є)?

Відповісти
Підтримати
Dou Dou 19.04.2023 23:59

Как вообще у взрослых людей есть время на цирк по типу код-ревью? Вникнуть по настоящему в чужую задачу это тоже самое что полдня сесть потратить.

Відповісти
Підтримати
Karina Kosenko
Victor Gubin Programmer 20.04.2023 00:14

По идее — это надо чтобы люди следили что делают другие ребята из команды, в случае если вы пересекаетесь где-то пойти договорится. Очень сильно помогает с разными мержами и т.д. До всех этих модных гитхабов и гитлабов — берешь арахис или кдифф3 и смотришь комиты. То что происходит явный показатель дезорганизации в проекте, лиду если он есть надо поставить на вид. А иногда это показатель устроенной внутренней конкуренции нездоровой, которая переросла в войну на всех уровнях и на код ревью она только начинает проявляться, но там конфликт может быть с очень не слабыми внутренними и весьма серьезными причинами финансового характера.

Відповісти
Підтримати
Dou Dou
Gennadiy Dubina Senior Software developer в DataArt 20.04.2023 00:22

Цирк? Только код ревью может обеспечить хоть какой-то шаринг знаний внутри команды. Бейте большие стори на кусочки и делайте. В PR изменений не больше 10-20 классов за раз. В зависимости от объема.
Всегда задачу можно декомпозировать и делать кусками.

Відповісти
Підтримати
Dou Dou
Dou Dou 20.04.2023 00:25
Цирк? Только код ревью может обеспечить хоть какой-то шаринг знаний внутри команды.

Ахахаха шаринг знаний))) Домен настолько огромный, знания уложены в гигабайты вордовских файлов, до терабайта видео. А один девелопер за 5 лет может даже за рамки своего модуля не выйти. И ему и не нужно оттуда вылазить, чтобы продолжать быть эффективным там где ему место.

Відповісти
Підтримати
Gennadiy Dubina
Sam Kerzhnerman General Manager в DomainOptions, Inc. 20.04.2023 00:28

ты щас как тот китаец, только со знаком минус

Відповісти
Підтримати
Dou Dou
Dou Dou 20.04.2023 00:34

Есть здравый смысл. Приходишь на готовую кодовую базу большую. Смотришь на нее недельку и пишешь все остальное не как тебе хочется, а как в этой кодовой базе. И не отсвечиваешь своими девиантными названиями переменных и отступами по 17 пробелов.

Відповісти
Підтримати
Sam Kerzhnerman
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 10:44
Ахахаха шаринг знаний))) Домен настолько огромный, знания уложены в гигабайты вордовских файлов, до терабайта видео. А один девелопер за 5 лет может даже за рамки своего модуля не выйти.

Ну если у вас bus factor меньше единицы, то я искренне сочувствую.

Відповісти
Підтримати
Dou Dou
Dou Dou 23.04.2023 14:02

У нас основной носитель знаний это отдельные специалисты и это вообще не девелоперы. Вот на них этот бас фактор может сработать. Девелопер же просто винтик, которому открывается картина постольку поскольку он работал сам долго с модулем, документацией модуля и консультировался с основным носителем знаний.

Відповісти
Підтримати
Valentin Nechayev
Valentin Nechayev архімаггриб в Дарницькі печери 23.04.2023 16:01

Ты можешь рассказывать как угодно кто основной носитель, а кто так, но важно — сколько времени искать спеца подходящего профиля и вводить его до того же уровня компетенции.
И как этот срок соотносится со сроком, допущенным для решения проблемы (а обычно это недели, если не меньше).

И если

он работал сам долго с модулем, документацией модуля и консультировался с основным носителем знаний

чего-то стоит, то мерять надо и по таким спецам.

Відповісти
Підтримати
Dou Dou
Victor Gubin Programmer 20.04.2023 01:13

Не всегда это имеет смысл. Скажем команда не особо большая. Выписывать все требования дотошно, тоже не особо времени есть — например через неделю надо показать proof of concept или MVP. Садятся пару senior-ов и фигачат типовой солюшн, черновой из пары тысяч строк. Куча тикетов в таком случае, это все на месяц задержат может даже больше. Конечно когда в проекте человек 5 и больше, вариантов уже особо нет — надо делить на маленькие формализованные задачи.

Відповісти
Підтримати
Gennadiy Dubina
Dmitry Bugay Java Team Lead 20.04.2023 16:43

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

Відповісти
Підтримати
Dou Dou
Олексій Хількевич 20.04.2023 12:04

личка нічого не значить

в будьякій групі людей автоматично будується кілька власних особливих негласних ієрархій з різних питань

оргструктура часто з цим не співпадає

Відповісти
Підтримати
Karina Kosenko
Victor Gubin Programmer 21.04.2023 01:03

Ну на сам перед вона зарплату значить, це офіційний показник кваліфікації яку компанія присвоює своему робітнику. Інша справа про наявність стандартів, та трактування цих стандартів — а ще бізнес вимоги відповіднітю. В нас 23 літній архітект чи вже з запозиченими назвами — principal, із якоїсь дуже модної технології, це дуже часто зустрічається, а от джуніор із 3 роками досвіду дуже рідко. Та для нашого архітекта 120 тисяч на рік — тобто $10 000 на місяць буде унікально хорошею зарплатнею, в двічі більшою за середню. В Google — $120 000 на рік, це зарплатня юніора, щоправда в долині різко нижча покупна спроможність грошей і 10 тисяч на місяць в Києві це далеко не те саме, що та сама сума в Сан-Франциско.

Відповісти
Підтримати
Олексій Хількевич
Pavel Ognev Senior software developer в GlobalLogic 25.04.2023 18:11

Ну джун в рев’юверах — це взагалі норм. В моїй команді завжди додаються новачки до всіх ревью, щоб більше дивилися, що навколо коїться.

Тут скоріше питання до «без нього не заапрувиш» та «він довше всіх на проєкті».
Впевнений, з проєктом щось дуже негаразд, і є привід тікати звідти.

Відповісти
Підтримати
Karina Kosenko
Антон Кучеренко 06.05.2023 11:49

Тобто людина, яка (виключно з контексту) понад 3 роки успішно скалює архітектуру, та особисто продовжує розробку, тільки цього конкретно взятого діючого проекта та користується великим авторитетом в команді — це джун?

Відповісти
Підтримати
Pavel Ognev

Підписатись на коментарі

Не підписуватись
Скористайтесь акаунтом
×
з умовами використання сайту і політикою конфіденційності.