Code Review

alex_127
Уже с Приветом
Posts: 7723
Joined: 29 Mar 2000 10:01
Location: Kirkland,WA

Re: Code Review

Post by alex_127 »

Херовимчик wrote: 28 Jul 2021 00:12 Я полтора года не вступала в этот бой. Но ситуацию довели до абсурда и призвали тяжёлую артиллерию :food:
Я честно не знаю как можно стать эффективным членом команды если средее время нахождения в ней 6 месяцев. Вы знаете? круто. Это не мое, я так не умею. меня не зовите и я вам не подойду.
User avatar
Komissar
Уже с Приветом
Posts: 64875
Joined: 12 Jul 2002 16:38
Location: г.Москва, ул. Б. Лубянка, д.2

Re: Code Review

Post by Komissar »

Херовимчик wrote: 28 Jul 2021 01:21
Komissar wrote: 28 Jul 2021 01:01
Херовимчик wrote: 28 Jul 2021 00:12 Я полтора года не вступала в этот бой. Но ситуацию довели до абсурда и призвали тяжёлую артиллерию :food:
забей. жизнь коротка.
Нельзя, велено разобраться и прекратить :sadcry: послать так, чтобы наконец-то дошло
прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
User avatar
Херовимчик
Уже с Приветом
Posts: 5284
Joined: 27 Sep 2008 21:48
Location: Moscow-Seattle-SFBA

Re: Code Review

Post by Херовимчик »

alex_127 wrote: 28 Jul 2021 01:29
Херовимчик wrote: 28 Jul 2021 00:12 Я полтора года не вступала в этот бой. Но ситуацию довели до абсурда и призвали тяжёлую артиллерию :food:
Я честно не знаю как можно стать эффективным членом команды если средее время нахождения в ней 6 месяцев. Вы знаете? круто. Это не мое, я так не умею. меня не зовите и я вам не подойду.
Я не знаю откуда вы сделали вывод о 6 месяцах? Средний tenure основных участников 5 лет. Контракторы приходят и уходят, из-за них расчитывать на сознательность не приходиться
Komissar wrote: 28 Jul 2021 01:51 [quote=Херовимчик post_id=7489210 time=<a href="tel:1627431695">1627431695</a> user_id=49996]
[quote=Komissar post_id=7489200 time=<a href="tel:1627430491">1627430491</a> user_id=8009]
[quote=Херовимчик post_id=7489193 time=<a href="tel:1627427537">1627427537</a> user_id=49996]
Я полтора года не вступала в этот бой. Но ситуацию довели до абсурда и призвали тяжёлую артиллерию :food:
забей. жизнь коротка.
[/quote]

Нельзя, велено разобраться и прекратить :sadcry: послать так, чтобы наконец-то дошло
[/quote]

прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
[/quote]

Раздоры
User avatar
KVA
Уже с Приветом
Posts: 5347
Joined: 03 Feb 1999 10:01
Location: NJ, USA

Re: Code Review

Post by KVA »

Херовимчик wrote: 28 Jul 2021 01:56
alex_127 wrote: 28 Jul 2021 01:29 прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
Раздоры
Сочувствую. Если нет авторитета или authority то ничего не выйдет.
alex_127
Уже с Приветом
Posts: 7723
Joined: 29 Mar 2000 10:01
Location: Kirkland,WA

Re: Code Review

Post by alex_127 »

Херовимчик wrote: 28 Jul 2021 01:56 прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
для этого совета совсем недостаточно данных.
как можно давать совет по ср если проблемы похоже глубже?
ответ верхнего уровня - нужен правильный хороший деятельный менеджер. поставить туда.
User avatar
Херовимчик
Уже с Приветом
Posts: 5284
Joined: 27 Sep 2008 21:48
Location: Moscow-Seattle-SFBA

Re: Code Review

Post by Херовимчик »

alex_127 wrote: 28 Jul 2021 02:10
Херовимчик wrote: 28 Jul 2021 01:56 прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
для этого совета совсем недостаточно данных.
как можно давать совет по ср если проблемы похоже глубже?
ответ верхнего уровня - нужен правильный хороший деятельный менеджер. поставить туда.
Вопрос никак уладить раздор. Вопрос о существующих практиках в других местах
User avatar
Херовимчик
Уже с Приветом
Posts: 5284
Joined: 27 Sep 2008 21:48
Location: Moscow-Seattle-SFBA

Re: Code Review

Post by Херовимчик »

KVA wrote: 28 Jul 2021 02:08
Херовимчик wrote: 28 Jul 2021 01:56
alex_127 wrote: 28 Jul 2021 01:29 прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
Раздоры
Сочувствую. Если нет авторитета или authority то ничего не выйдет.
Ну вот авторитетом и будем мочить authority :wink:
User avatar
Komissar
Уже с Приветом
Posts: 64875
Joined: 12 Jul 2002 16:38
Location: г.Москва, ул. Б. Лубянка, д.2

Re: Code Review

Post by Komissar »

Херовимчик wrote: 28 Jul 2021 02:21
alex_127 wrote: 28 Jul 2021 02:10
Херовимчик wrote: 28 Jul 2021 01:56 прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
для этого совета совсем недостаточно данных.
как можно давать совет по ср если проблемы похоже глубже?
ответ верхнего уровня - нужен правильный хороший деятельный менеджер. поставить туда.
Вопрос никак уладить раздор. Вопрос о существующих практиках в других местах
мать, пора валить
User avatar
KVA
Уже с Приветом
Posts: 5347
Joined: 03 Feb 1999 10:01
Location: NJ, USA

Re: Code Review

Post by KVA »

Херовимчик wrote: 28 Jul 2021 02:21
alex_127 wrote: 28 Jul 2021 02:10
Херовимчик wrote: 28 Jul 2021 01:56 прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
для этого совета совсем недостаточно данных.
как можно давать совет по ср если проблемы похоже глубже?
ответ верхнего уровня - нужен правильный хороший деятельный менеджер. поставить туда.
Вопрос никак уладить раздор. Вопрос о существующих практиках в других местах
Ну практики как бы очевидны. :pain1: От полной анархии и вольницы, до увольнения за сломанный билд (наверное где-то и такое есть). По статистике скорее всего практики где-то посередине. IMHO знание того что делается в другом месте никак не поможет решить вашу конкретную проблему. Ваша проблема не в технической области, а в области менеджмента и authority. Короче, кулаком надо стукнуть, сказать как надо (вы и сами сможете сообразить как надо) и наказывать за как не надо (а вот тут у вас authority-то и нет). Так что проблема не решаемая на вашем уровне.
nyekimov
Уже с Приветом
Posts: 2761
Joined: 11 Jul 2015 19:01
Location: Chicago

Re: Code Review

Post by nyekimov »

Был в разных ситуациях. Отсутсвие pull request review в команде больше одного человека считаю красным флагом. Сломал поломал хорошо ровно тогда, когда ты один и надо убирать только за собой и соответственно все карты на руках сделать это оперативно.

Чем больше команда и соотвественно проект тем, считаю, важней код ревью. Но должна быть какая то золотая середина. Например стилистика должна проверяться линтерами. CI тоже должен иметь это встроенно и репортовать о проблемах как только открывается pull request. У нас нет никаких Тулов, есть формат: jira ticket id period meaningful change description.
Я вообще ненавижу какие либо тулы кроме cli вечно эти тулы начинают тормозить в итоге.
С синдромом вахтёра тоже сталкивался. Когда человек будет придираться даже к грамматике в комментарии к тесту, который описывает что вообще тест должен делать.

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

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

У нас кстати в большой команде самая большая проблема в том, что на все этапы пока так и не успели прикрутить ci и скажем релиз н работает с новыми изменениями, а вот н+1 они ломают. И потом этот н + 1 вечно встаёт из-за изменений снизу. Но ответственная команда(ы) занята релизом н, сроки горят и все дела. Им некогда проверять все резилы сверху, а ci только в процессе разработки ночных билдов всего и всея. Где то они уже есть и вечно падают, опять же никому пока до этого дела нет. Так и живем.
nyekimov
Уже с Приветом
Posts: 2761
Joined: 11 Jul 2015 19:01
Location: Chicago

Re: Code Review

Post by nyekimov »

KVA wrote: 28 Jul 2021 02:38
Херовимчик wrote: 28 Jul 2021 02:21
alex_127 wrote: 28 Jul 2021 02:10
Херовимчик wrote: 28 Jul 2021 01:56 прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
для этого совета совсем недостаточно данных.
как можно давать совет по ср если проблемы похоже глубже?
ответ верхнего уровня - нужен правильный хороший деятельный менеджер. поставить туда.
Вопрос никак уладить раздор. Вопрос о существующих практиках в других местах
Ну практики как бы очевидны. :pain1: От полной анархии и вольницы, до увольнения за сломанный билд (наверное где-то и такое есть). По статистике скорее всего практики где-то посередине. IMHO знание того что делается в другом месте никак не поможет решить вашу конкретную проблему. Ваша проблема не в технической области, а в области менеджмента и authority. Короче, кулаком надо стукнуть, сказать как надо (вы и сами сможете сообразить как надо) и наказывать за как не надо (а вот тут у вас authority-то и нет). Так что проблема не решаемая на вашем уровне.
Хороший коммент; поддерживаю.

Если sme выкатить идеальный pull request, то в чем проблема?

Чаще я вижу проблемы, когда человек не удосуживается исправляться, поработать над читательбностью. Оперативно не реагирует на вопросы или просьбы исправить код. Вместо этого бежит жаловаться, что его прессуют. И тут уже очень сильно зависит от менеджмента, кто то соображает поставить «спеца» на место, а некоторые менеджера даже начинают ругать ревьюверов. И вот для этого кстати не мешал бы свод правил, чтобы все знали критерии хорошего pull request -а. Свод правил будет позволять поставить и «вахтёра» на место.
User avatar
Херовимчик
Уже с Приветом
Posts: 5284
Joined: 27 Sep 2008 21:48
Location: Moscow-Seattle-SFBA

Re: Code Review

Post by Херовимчик »

Komissar wrote: 28 Jul 2021 02:36
Херовимчик wrote: 28 Jul 2021 02:21
alex_127 wrote: 28 Jul 2021 02:10
Херовимчик wrote: 28 Jul 2021 01:56 прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
для этого совета совсем недостаточно данных.
как можно давать совет по ср если проблемы похоже глубже?
ответ верхнего уровня - нужен правильный хороший деятельный менеджер. поставить туда.
Вопрос никак уладить раздор. Вопрос о существующих практиках в других местах
мать, пора валить
Да ладно, и не таких мочили :mrgreen:
User avatar
Херовимчик
Уже с Приветом
Posts: 5284
Joined: 27 Sep 2008 21:48
Location: Moscow-Seattle-SFBA

Re: Code Review

Post by Херовимчик »

KVA wrote: 28 Jul 2021 02:38
Херовимчик wrote: 28 Jul 2021 02:21
alex_127 wrote: 28 Jul 2021 02:10
Херовимчик wrote: 28 Jul 2021 01:56 прекратить... что именно? Поломки билдов, баги в софте или раздоры между группами?
для этого совета совсем недостаточно данных.
как можно давать совет по ср если проблемы похоже глубже?
ответ верхнего уровня - нужен правильный хороший деятельный менеджер. поставить туда.
Вопрос никак уладить раздор. Вопрос о существующих практиках в других местах
Ну практики как бы очевидны. :pain1: От полной анархии и вольницы, до увольнения за сломанный билд (наверное где-то и такое есть). По статистике скорее всего практики где-то посередине. IMHO знание того что делается в другом месте никак не поможет решить вашу конкретную проблему. Ваша проблема не в технической области, а в области менеджмента и authority. Короче, кулаком надо стукнуть, сказать как надо (вы и сами сможете сообразить как надо) и наказывать за как не надо (а вот тут у вас authority-то и нет). Так что проблема не решаемая на вашем уровне.
У нас не принято пускать в ход authority (обе оппозиции имеют одинаковый вес тут). Верх инженерной мысли как раз найти совместное решение. И тут чужой опыт хороший data point
User avatar
Херовимчик
Уже с Приветом
Posts: 5284
Joined: 27 Sep 2008 21:48
Location: Moscow-Seattle-SFBA

Re: Code Review

Post by Херовимчик »

nyekimov wrote: 28 Jul 2021 02:49 Был в разных ситуациях.
Вот, вы меня хорошо поняли! Одно дело пара инвалидов и 3 PR в месяц, другое дело пол сотни народу, с десятками PR в день и частыми релизами
User avatar
KVA
Уже с Приветом
Posts: 5347
Joined: 03 Feb 1999 10:01
Location: NJ, USA

Re: Code Review

Post by KVA »

Херовимчик wrote: 28 Jul 2021 03:16 У нас не принято пускать в ход authority (обе оппозиции имеют одинаковый вес тут). Верх инженерной мысли как раз найти совместное решение. И тут чужой опыт хороший data point
Тогда усядемся получше и бум наблюдать шоу. :food: :D

Return to “Работа и Карьера в IT”