Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>Часто бывают спорные ситуации во время code review. Это либо опыт у людей разный, либо дело вкуса, какие-то "холиварные" темы A_L>Например, писать/не писать комментарии,
Если я прошу написать комментарий значит или мне не очевидно зачем этот код нужен или что он делает(надо написать), или по стилю кодирования у нас тут должен быть комментарий, даже если он очевидный из разряда // Запрос ФИО A_L>название переменных,
Должно быть по стилю кодирования A_L>использовать/не использовать var и т.д.. A_L>Список можно продолжать до бесконечности
Максимально вынести в стиль кодирования, желательно с обоснованием
A_L>Это те моменты, которые никак качественно не влияют на работу приложения. Т.е. например, это не о том как код используется больше/меньше ресурсов A_L>Team leader проверяет, вносит свои правки, а у автора кода другое мнение как лучше для проекта
То что ты описал это чисто стиль кодирования, он составляется с участниками команды и потом все его придерживаются, после того как попишешь в разных стилях, просто будешь принимать то как исторически сложилось в проекте
A_L>Как поступать в подобных ситуациях — соглашаться с мнением team leader или продолжать отстаивать свое? A_L>Обратиться к кому-то "выше" за разъяснениями нет возможности
1) Есть ли общий стиль кодирования? Если нет, предложи сделать его, и вынести его для обсуждения всей командой
2) После стиля добавить линтер который будет поддерживать стиль
3) Если код не по стилю, сборка не должна собираться, в идеале настроить чтобы у всех был локальный прогон перед пушем в репозиторий.
Вот после этого скорей всего будут замечания по существу, из не знаю, в начале функции сначала сделай обработку граничных условий, а только потом логику, шум из того что ты описал должен уйти.
Re[2]: Как решать конфликтные ситуации при code review?
Здравствуйте, Igore, Вы писали:
I>Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>>Часто бывают спорные ситуации во время code review. Это либо опыт у людей разный, либо дело вкуса, какие-то "холиварные" темы A_L>>Например, писать/не писать комментарии, I>Если я прошу написать комментарий значит или мне не очевидно зачем этот код нужен или что он делает(надо написать), или по стилю кодирования у нас тут должен быть комментарий, даже если он очевидный из разряда // Запрос ФИО A_L>>название переменных, I>Должно быть по стилю кодирования A_L>>использовать/не использовать var и т.д.. A_L>>Список можно продолжать до бесконечности I>Максимально вынести в стиль кодирования, желательно с обоснованием
A_L>>Это те моменты, которые никак качественно не влияют на работу приложения. Т.е. например, это не о том как код используется больше/меньше ресурсов A_L>>Team leader проверяет, вносит свои правки, а у автора кода другое мнение как лучше для проекта I>То что ты описал это чисто стиль кодирования, он составляется с участниками команды и потом все его придерживаются, после того как попишешь в разных стилях, просто будешь принимать то как исторически сложилось в проекте
A_L>>Как поступать в подобных ситуациях — соглашаться с мнением team leader или продолжать отстаивать свое? A_L>>Обратиться к кому-то "выше" за разъяснениями нет возможности I>1) Есть ли общий стиль кодирования? Если нет, предложи сделать его, и вынести его для обсуждения всей командой I>2) После стиля добавить линтер который будет поддерживать стиль I>3) Если код не по стилю, сборка не должна собираться, в идеале настроить чтобы у всех был локальный прогон перед пушем в репозиторий. I>Вот после этого скорей всего будут замечания по существу, из не знаю, в начале функции сначала сделай обработку граничных условий, а только потом логику, шум из того что ты описал должен уйти.
Не всегда руководство принимает предложения по улучшению продукта. Например, сотрудник предлагает исходя из опыта как можно улучшить. Руководство формально соглашается, но в работу это не идет
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
I>>Вот после этого скорей всего будут замечания по существу, из не знаю, в начале функции сначала сделай обработку граничных условий, а только потом логику, шум из того что ты описал должен уйти. A_L>Не всегда руководство принимает предложения по улучшению продукта. Например, сотрудник предлагает исходя из опыта как можно улучшить. Руководство формально соглашается, но в работу это не идет
Ну смотря что за доработки, если соглашается надо в дорожную карту вносить, а не поговорили и забыли, или обговорить что это сделаем вон с той задачей.
У нас целая страница на wiki есть с предложениями по улучшению, которые было бы неплохо сделать, но работать не мешает, а клиент за такие доработки не заплатит, берутся оттуда мелкие задачи когда человек новый в проект приходит как ознакомление и улучшение проекта. А большие задачи которые хотелось бы, только с обоснованием или в связке с пожеланием клиента который готов за доработки платить деньги.
Re: Как решать конфликтные ситуации при code review?
Здравствуйте, Pzz, Вы писали:
vsb>>Если ревью от вышестоящего товарища, надо делать как он сказал. Если ревью от коллеги, который придирается по мелочам, тут можно и поспорить. Хотя может быть лучше сделать как он просит и забить, если он не просит чего-то явно плохого и это не отнимет много времени.
Pzz>А что, вышестоящий товарищ более компетентен по определению?
Конечно. На то иерархия и выстроена.
Re: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
AL>Как поступать в подобных ситуациях — соглашаться с мнением team leader или продолжать отстаивать свое?
С одной стороны вы говорите что TL цепляется за var и по комментариям, и аргументируете «программа ж работает». А с другой стороны — вы спрашиваете «продолжать ли вам отстаивать свое»? Определите для себя что для вас важнее — уметь работать в команде и уметь писать комментарии и внимать критику старших коллег при code review или продолжать отстаивать свои убеждения, которое вам навязано из ютубов и форумов (потому что в книга этого точно нет)?
Наконец, вы же с лидом делаете одно и тоже дело, вы достигнете большего успеха — если будете работать с лидом вместе, чем конфликтовать.
...<< Dementor 1.5.4 ✪ Lets Play a Game ⚁⚂⚂⚃⚄>>
Re[2]: Как решать конфликтные ситуации при code review?
Здравствуйте, GarryIV, Вы писали:
GIV>Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>>Team leader проверяет, вносит свои правки, а у автора кода другое мнение как лучше для проекта
GIV>Ты как "тварь дрожащая" должен выполнять, иначе бы не спрашивал тут.
Почему "тварь дрожащая"? К тебе так обращаются?
Я спрашиваю что-либо когда мне хочется или удобно
Re[2]: Как решать конфликтные ситуации при code review?
Здравствуйте, r0nd, Вы писали:
R>Здравствуйте, Aleksei_Lekomtsev, Вы писали:
AL>>Как поступать в подобных ситуациях — соглашаться с мнением team leader или продолжать отстаивать свое?
R>С одной стороны вы говорите что TL цепляется за var и по комментариям, и аргументируете «программа ж работает». А с другой стороны — вы спрашиваете «продолжать ли вам отстаивать свое»? Определите для себя что для вас важнее — уметь работать в команде и уметь писать комментарии и внимать критику старших коллег при code review или продолжать отстаивать свои убеждения, которое вам навязано из ютубов и форумов (потому что в книга этого точно нет)?
R>Наконец, вы же с лидом делаете одно и тоже дело, вы достигнете большего успеха — если будете работать с лидом вместе, чем конфликтовать.
С одной стороны вы говорите что TL цепляется за var и по комментариям, и аргументируете «программа ж работает».
Я так не аргументировал
уметь работать в команде и уметь писать комментарии
Я умею работать в команде и писать комментарии и люблю слушать конструктивную критику по существу
которое вам навязано из ютубов и форумов
Это не правда
вы достигнете большего успеха — если будете работать с лидом вместе, чем конфликтовать
Смотря что считать успехом
Re: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>Часто бывают спорные ситуации во время code review. Это либо опыт у людей разный, либо дело вкуса, какие-то "холиварные" темы A_L>Например, писать/не писать комментарии, название переменных, использовать/не использовать var и т.д.. Список можно продолжать до бесконечности A_L>Это те моменты, которые никак качественно не влияют на работу приложения. Т.е. например, это не о том как код используется больше/меньше ресурсов
Если речь идет о стилистических моментах, то спор имеет смысл только если обсуждаются именно они, а не конкретный pull request.
Если в команде они не формализован никак, то стоит конфликтные моменты расписать в wiki. Ты возможно не будешь с ними согласен, но это обычно "not а hill to die on". Извиняюсь за поговорку, но на ум ничего рускоязычного не приходит.
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
AL>Я так не аргументировал
Давайте попробуем с другого конца. Если вы не аргументируете, то я буду аргументировать. Есть пояснительная записка к каждой роли в проекте/продукте. У лида среди прочего предписано, что он отвечает за code review. Ни вы, ни другие члены команды, не скрам мастер/PM или начальник начальника, а лид отвечает и только лид. Второе зафиксируем, требования по ревизии кода почти всегда отличаются от проекта к проекту. И они же формируются лидом и артикулируются также лидом. Более того, сам процесс адаптации/изменения требований к code review непрерывен и постоянен. Зачем? Потому что технологии и языки развиваются и меняются. Поэтому если, например, стек проектов предполагает несколько версий с обратной совместимостью к версиям — могут появлятся и странные (на первый взгляд) требования к ревизии кода.
Вы переходите в другую команду — и уже в другой команде могут быть другие требования к ревизии кода (ниже напишу вам реальный кейс¹). И здесь нет никакого холивара, о котором вы говорите. Ибо, как лид сказал — так и есть. Точка. Потому что это его зона отвественности. Это обязаность лида, не ваша. Какой холивар? Да, стоит отметить что лид зачастую делегирует на кого-то еще код ревью (на усмотрение самого лида) синьору какому-то или мидлу, которого он видит в будущем синьором, но как я сказал раньше, только лид отвечает за процесс ревизии кода.
AL>
AL>которое вам навязано из ютубов и форумов
AL>Это не правда
А откуда у вас тогда такое мнение? Откуда вы это почерпнули? Какие книги? Или в ВУЗе вам это сообщили?
AL>Смотря что считать успехом
Это неуспех, только наоборот. Конфликт с лидом — я называю неуспехом.
⸻
¹ про обещанный кейс, часто бывает нужно сделать правки, которые затрагивают другие проекты. И как правило проводят ревизию правок лиды тех проектов, куда делались правки. И даже если вы директор департамента/CTO/PMO пофиг — правите так, как вам скажет этот лид. (Было что правилил комментарии в гите). Потому что нужно было их специальным образом оформлять — далеим от общепринятого. Хотя номанально, он (этот лид) может быть вашим прямым подчиненным. Потому что в code review — главный тот, чей проект. А это лид.
...<< Dementor 1.5.4 ✪ Lets Play a Game ⚀⚂⚃⚅⚅>>
Re[4]: Как решать конфликтные ситуации при code review?
Здравствуйте, r0nd, Вы писали:
R>Здравствуйте, Aleksei_Lekomtsev, Вы писали:
AL>>Я так не аргументировал
R>Давайте попробуем с другого конца. Если вы не аргументируете, то я буду аргументировать. Есть пояснительная записка к каждой роли в проекте/продукте. У лида среди прочего предписано, что он отвечает за code review. Ни вы, ни другие члены команды, не скрам мастер/PM или начальник начальника, а лид отвечает и только лид. Второе зафиксируем, требования по ревизии кода почти всегда отличаются от проекта к проекту. И они же формируются лидом и артикулируются также лидом. Более того, сам процесс адаптации/изменения требований к code review непрерывен и постоянен. Зачем? Потому что технологии и языки развиваются и меняются. Поэтому если, например, стек проектов предполагает несколько версий с обратной совместимостью к версиям — могут появлятся и странные (на первый взгляд) требования к ревизии кода.
R>Вы переходите в другую команду — и уже в другой команде могут быть другие требования к ревизии кода (ниже напишу вам реальный кейс¹). И здесь нет никакого холивара, о котором вы говорите. Ибо, как лид сказал — так и есть. Точка. Потому что это его зона отвественности. Это обязаность лида, не ваша. Какой холивар? Да, стоит отметить что лид зачастую делегирует на кого-то еще код ревью (на усмотрение самого лида) синьору какому-то или мидлу, которого он видит в будущем синьором, но как я сказал раньше, только лид отвечает за процесс ревизии кода.
AL>>
AL>>которое вам навязано из ютубов и форумов
AL>>Это не правда
R>А откуда у вас тогда такое мнение? Откуда вы это почерпнули? Какие книги? Или в ВУЗе вам это сообщили?
AL>>Смотря что считать успехом
R>Это неуспех, только наоборот. Конфликт с лидом — я называю неуспехом.
R>⸻ R>¹ про обещанный кейс, часто бывает нужно сделать правки, которые затрагивают другие проекты. И как правило проводят ревизию правок лиды тех проектов, куда делались правки. И даже если вы директор департамента/CTO/PMO пофиг — правите так, как вам скажет этот лид. (Было что правилил комментарии в гите). Потому что нужно было их специальным образом оформлять — далеим от общепринятого. Хотя номанально, он (этот лид) может быть вашим прямым подчиненным. Потому что в code review — главный тот, чей проект. А это лид.
А откуда у вас тогда такое мнение? Откуда вы это почерпнули? Какие книги? Или в ВУЗе вам это сообщили?
От более опытных коллег с других команд. Все в сравнении
Книги разные бывают. Бывают книги в которых много "упрощений". Были проекты, в которых много оптимизаций, книжные примеры для того бизнеса бы не подошли
Re[4]: Как решать конфликтные ситуации при code review?
Здравствуйте, Sharov, Вы писали:
S>Ну а что в этом хорошего, если у людей, например, принято функциональность из 3х классов размазывать по ddd?
Не очень понимаю, как "писать/не писать комментарии, название переменных, использовать/не использовать var и т.д." из стартового сообщения превратилось в "принято функциональность из 3х классов размазывать по ddd"...
S>Тот факт что так принято или деды так делали и мы будем -- ну такой себе аргумент.
Конфликт "один против всей команды" тоже продуктивности не добавит.
Но, по крайней мере, при наличии плохих задокументированных стилей кодирования, у тебя будет аргумент для спора.
И можно будет идти по пунктам, объясняя, почему так делать плохо. Но не на code review.
А превращать code review в религиозные войны по тому, как оформлять код...
Это не задача code review — определять, как оформлять код "правильно".
Задача code review — подтвердить, что код оформлен "правильно" с точки зрения уже принятых в проекте стандартов.
S>Тут через себя переступать приходится.
Чтобы что? Какова цель переступания?
"Потерял дар речи за зря"(с).
Re[5]: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>Специалист, наверное хочет чтобы было как лучше
А тимлид, всё-таки, хочет как хуже или он не специалист?
A_L>Вот программист и делает свою работу, предлагая свои решения как будет лучше для компании. Это не связано с желанием работать/не работать
Нежелание убрать лишние комменты или добавить недостающие, переименовать переменные так, как понятно команде — это что?
Как это существенно улучшает позицию компании на рынке?
A_L>Вы я так понимаю программист или у вас другая профессия?
А ты оверквоттер? Прояви уважение к собеседникам, отформатируй свои сообщения так, чтобы их читать было удобно.
К коду ты тоже так относишься, кстати?
"Потерял дар речи за зря"(с).
Re[3]: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>Я умею работать в команде и писать комментарии и люблю слушать конструктивную критику по существу
А что "по существу", а что нет — решаешь в команде только ты, да?
Типа "качественно не влияют на работу приложения" — это существенно, это ты уже написал.
А то, что твой код понять сложно из-за того, что название переменных неудачные и вводят в заблуждение, например — это не существенно, да?
И что ты поленился написать комментарий к сложной, неочевидной логике — это тоже на работу приложения не влияет, значит не существенно?
А то, что ты спустя полгода сам не поймёшь, что там к чему без комментария и с хреновыми названиями переменных, а пофиксить надо срочно, клиенты теряют деньги из-за изменения обстоятельств, например, — это ерунда, "не влияет на работу приложения". Я всё правильно понял?
"Потерял дар речи за зря"(с).
Re[5]: Как решать конфликтные ситуации при code review?
Здравствуйте, SkyDance, Вы писали:
SD>Нет. Диктатура может быть только до определенного уровня деталей. Диктатура вида "здесь можно комменты, а тут нельзя", сделает плохо.
Посмотри как он здесь, на форуме, оформляет свои сообщения.
Оставить как есть, или, всё-таки, призвать диктатора-модератора?
Есть ли вероятность, что его код по небрежности оформления не особо отличается от сообщений на форуме?
ИМХО, немалая.
"Потерял дар речи за зря"(с).
Re[6]: Как решать конфликтные ситуации при code review?
Здравствуйте, _ABC_, Вы писали:
_AB>Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>>Специалист, наверное хочет чтобы было как лучше _AB>А тимлид, всё-таки, хочет как хуже или он не специалист?
A_L>>Вот программист и делает свою работу, предлагая свои решения как будет лучше для компании. Это не связано с желанием работать/не работать _AB>Нежелание убрать лишние комменты или добавить недостающие, переименовать переменные так, как понятно команде — это что? _AB>Как это существенно улучшает позицию компании на рынке?
A_L>>Вы я так понимаю программист или у вас другая профессия? _AB>А ты оверквоттер? Прояви уважение к собеседникам, отформатируй свои сообщения так, чтобы их читать было удобно. _AB>К коду ты тоже так относишься, кстати?
A_L>>Специалист, наверное хочет чтобы было как лучше _AB>А тимлид, всё-таки, хочет как хуже или он не специалист?
Читать на русском умеешь?
_AB>Нежелание убрать лишние комменты или добавить недостающие, переименовать переменные так, как понятно команде — это что? _AB>Как это существенно улучшает позицию компании на рынке?
Это субъективно, тут ты мне ничего не докажешь
_AB>А ты оверквоттер? Прояви уважение к собеседникам, отформатируй свои сообщения так, чтобы их читать было удобно. _AB>К коду ты тоже так относишься, кстати?
Ты своим указывай как что делать
Re[4]: Как решать конфликтные ситуации при code review?
Здравствуйте, _ABC_, Вы писали:
_AB>Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>>Я умею работать в команде и писать комментарии и люблю слушать конструктивную критику по существу _AB>А что "по существу", а что нет — решаешь в команде только ты, да?
С чего ты это решил?)
_AB>А то, что твой код понять сложно из-за того, что название переменных неудачные и вводят в заблуждение, например — это не существенно, да?
Почему неудачные? Может кто-то читает плохо и вводится в заблуждение? Бесполезно тут что-то тебе писать. Много промышленного кода написано в разных командах,
все кто хотел понять — понимали. Специально не придумывал названия переменных чтобы никто не понял
_AB>И что ты поленился написать комментарий к сложной, неочевидной логике — это тоже на работу приложения не влияет, значит не существенно?
Почему сложная неочевидная логика? Почему поленился? Обычный код, никакого rocket science
_AB>А то, что ты спустя полгода сам не поймёшь, что там к чему без комментария и с хреновыми названиями переменных, а пофиксить надо срочно, клиенты теряют деньги из-за изменения обстоятельств, например, — это ерунда, "не влияет на работу приложения". Я всё правильно понял?
Почему спустя полгода я не пойму? Почему хреновые названия?
Извини, ты странный и очень аггресивный. Навыдумывал кучу всего, что к реальности не имеет никакого отношения. Дальше обвинил в этом абсолютно незнакомого человека и навешал на некого кучу ярлыков.
Ты как будто живешь в своем собственном иллюзорном мире, где события происходят по одному тебе известному сценарию и при этом решил, что можешь хамить людям
