Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>Часто бывают спорные ситуации во время code review. Это либо опыт у людей разный, либо дело вкуса, какие-то "холиварные" темы A_L>Например, писать/не писать комментарии, название переменных, использовать/не использовать var и т.д.. Список можно продолжать до бесконечности A_L>Это те моменты, которые никак качественно не влияют на работу приложения. Т.е. например, это не о том как код используется больше/меньше ресурсов
Должен быть документ по coding style и, как продолжение, линтеры и форматтеры, прехуки и прочие "радости". A_L>Team leader проверяет, вносит свои правки, а у автора кода другое мнение как лучше для проекта
Обсудить голосом/лично, если не можешь убедить, значит твоя позиция не достаточно сильная или ты чего-то не понимаешь/не видишь. Дальше делаешь как просят. A_L>Как поступать в подобных ситуациях — соглашаться с мнением team leader или продолжать отстаивать свое?
Зачем? Какая цель этого? Ты работаешь по найму за з/п. A_L>Обратиться к кому-то "выше" за разъяснениями нет возможности
У Макконела было. Если просят поправить тривиальные вещи, то лучше поправить как просят и всё.
Sic luceat lux!
Re: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>Часто бывают спорные ситуации во время code review. Это либо опыт у людей разный, либо дело вкуса, какие-то "холиварные" темы A_L>Например, писать/не писать комментарии, название переменных, использовать/не использовать var и т.д.. Список можно продолжать до бесконечности A_L>Это те моменты, которые никак качественно не влияют на работу приложения. Т.е. например, это не о том как код используется больше/меньше ресурсов
Если это непринципиальные моменты типа упомянутых, проще всего использовать code style,
желательно автоматически (набор правил, которые энфорсятся прямо в IDE, чтобы до review эти вопросы даже не доходили). Линтеры, рулесеты, вот это вот.
То есть, чтобы код написанный не по правилам, просто не компилировался. Займись лучше этим чем препирательством.
Стандартная же тема — делаешь в курсаче пару намеренных косяков — препод их с чувством выполненного долга находит, ты правишь, тема закрыта. Потому что иначе к чему прикопаться — все равно найдет, а переделывать будет больше.
A_L>Team leader проверяет, вносит свои правки, а у автора кода другое мнение как лучше для проекта
Лида из кодеров начальник прокачивал? Хороший лид должен заниматься разруливанием конфликтов между сотрудниками (грубо говоря, Вася вчера сделал Пете 3 уступки, значит сегодня Петя должен сделать Васе 3, чтобы был паритет) и коммуникацией с PM, чтобы на погромистов не лился поток меняющихся каждую неделю требований, которые надо было сделать вчера.
A_L>Как поступать в подобных ситуациях — соглашаться с мнением team leader или продолжать отстаивать свое?
Открыть в голове счет. 10 часов потратил на написание кода, 1 час — на внесение правок, какими бы дурацкими они не были. Считай это издержками производства и успокойся. Если же выходит 5 часов правок на каждый час работы, то начинать искать другое место.
У меня так было — сидишь, продумываешь ортогональную декомпозицию, делаешь красивые уровни абстракции, а на ревью начинается — тут переименовать, там break вместо return, здесь аргументы поменять местами, и сиди переделывай все по нескольку раз. А что код делает, люди понимать даже не пытаются. Уволился — полегчало. Проект потом закрыли.
Re[2]: Как решать конфликтные ситуации при code review?
Здравствуйте, novitk, Вы писали:
N>Если в команде они не формализован никак, то стоит конфликтные моменты расписать в wiki. Ты возможно не будешь с ними согласен, но это обычно "not а hill to die on". Извиняюсь за поговорку, но на ум ничего рускоязычного не приходит.
+1
"Не стоит выеденного яйца".
Re[7]: Как решать конфликтные ситуации при code review?
[...куча оверквотинга поскипана...] _AB>>А ты оверквоттер? Прояви уважение к собеседникам, отформатируй свои сообщения так, чтобы их читать было удобно. A_L>Ты своим указывай как что делать
А ты так ничего и не понял...
Символично!
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Re[5]: Как решать конфликтные ситуации при code review?
Здравствуйте, Quebecois, Вы писали:
Q>Стандартная же тема — делаешь в курсаче пару намеренных косяков — препод их с чувством выполненного долга находит, ты правишь, тема закрыта. Потому что иначе к чему прикопаться — все равно найдет, а переделывать будет больше.
была гениальная история про известного старинного художника, который рисовал портреты всякой знати на заказ. и задолбали они его в конец, ибо принять сразу работу им было западло и они придирались, просили вдруг переписать их в другом костюме поверх старого и т.п. И художник придумал фишку — он в углу каждой картины рисовал маленькую зеленую собачку жутко противного вида, когда приходили заказчики, они первым делом замечали собачку и начинали орать «Что это за жуткая зеленая тварь?!» и художник говорил — «Ок, собачку убираем, все остальное в порядке, да?» Ну и они уже обессиленные и охреневшие от собачки соглашались
...<< Dementor 1.5.4 ✪ Lets Play a Game ⚂⚂⚂⚄⚅>>
Re[5]: Как решать конфликтные ситуации при code review?
Здравствуйте, Sharov, Вы писали:
S>Для простейших вещей из 3-4 классов использовать DI контейнер. И DDD натягивать к месту и к не.
1. Какое это имеет отношение к теме обсуждения?
2. Ты можешь попробовать переубедить всю команду, привыкшую так писать, конечно. Но сдаётся мне, что не получится, если тим лид за них. Проще найти другую команду, ИМХО.
"Потерял дар речи за зря"(с).
Re[5]: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
_AB>>А что "по существу", а что нет — решаешь в команде только ты, да? A_L>С чего ты это решил?)
С того, что ты так и пишешь прямым текстом.
автору как специалисту видится, что его решение лучше для проекта
Соответственно, всё, что не совпадает с мнением "автора, как специалиста" автоматически неконструктивно и не по существу.
A_L>Почему неудачные?
По мнению других членов команды, которые читают твой код.
A_L>Может кто-то читает плохо и вводится в заблуждение?
Может, например, ты? Не допускаешь такой мысли?
A_L>Бесполезно тут что-то тебе писать. Много промышленного кода написано в разных командах, A_L>все кто хотел понять — понимали.
Вопрос в том, сколько они при этом усилий затрачивали и вникали в логику кода, пытаясь понять, почему переменная названа так, а результат другой, например.
A_L>Специально не придумывал названия переменных чтобы никто не понял
Если бы кто-то специально такие названия придумывал, его надо было бы уволить.
А неудачные названия обычное дело. Особенно, если английский не родной.
Буквально сегодня просил одного программера переименовать в названиях методов Attachments на Files.
Он мне в ответ — я тогда на Objects поменяю, т.к. платформа, к которой методы обращаются, так называет файлы.
А посмотреть, что все другие методы, которые уже существуют в данном класе несколько лет, используют Files, человек не удосужился.
Тоже специалист, тоже виднее...
_AB>>И что ты поленился написать комментарий к сложной, неочевидной логике — это тоже на работу приложения не влияет, значит не существенно? A_L>Почему сложная неочевидная логика? Почему поленился? Обычный код, никакого rocket science
Я тебе условно гипотетическую ситуацию предлагаю в качестве примера того, как комментарии могут качественно сказываться. В виде вопроса.
Но, если ты никогда не писал ничего сложного и неочевидного и не возвращался к этому куску кода спустя заметное время, тогда, конечно, суть вопроса ты понять не можешь.
A_L>Извини, ты странный и очень аггресивный. Навыдумывал кучу всего, что к реальности не имеет никакого отношения.
Извини, но это ты очень странный.
Я привёл тебе примеры, когда "писать/не писать комментарии, название переменных"(с) могут качественно влиять на работу приложения и работу команды. Примеры из реальной практики, кстати.
A_L>Дальше обвинил в этом абсолютно незнакомого человека и навешал на некого кучу ярлыков.
Ты не второй ник мразеца 19? Тот тоже неспособен в простейшую абстракцию...
И я тебя "обвинил" ровно в одном. Я констатировал твоё полное неуважение ко всем участникам форума, выражающееся в отвратительном форматировании твоих сообщений и куче оверквоттинга. И предположил, что к своему коду ты относишься точно так же, т.к. обычно люди не меняют привычки работы над своими текстами в зависимости от контектста.
"Потерял дар речи за зря"(с).
Re[8]: Как решать конфликтные ситуации при code review?
Всему свое время, парень молодой, горячи (я надеюсь). А вообще 100% — отношение с руководителю с конкрутных позиций ни к чем хорошему не приводит, ибо нарушает суть вещей. Вероятность успешного хаджакингда в общем случае околоноля.
Re: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>Часто бывают спорные ситуации во время code review. Это либо опыт у людей разный, либо дело вкуса, какие-то "холиварные" темы A_L>Например, писать/не писать комментарии, название переменных, использовать/не использовать var и т.д.. Список можно продолжать до бесконечности A_L>Это те моменты, которые никак качественно не влияют на работу приложения. Т.е. например, это не о том как код используется больше/меньше ресурсов
Эти моменты сильно влияют на работу команды: на сколько удобно и приятно будет другим понимать и поддерживать написанное. На мой, например, взгляд это важный показатель "качества".
A_L>Team leader проверяет, вносит свои правки, а у автора кода другое мнение как лучше для проекта A_L>Как поступать в подобных ситуациях — соглашаться с мнением team leader или продолжать отстаивать свое?
Если этот человек "главный", в смысле, что он в конце концов отвечает за обсуждаемый код, то, тем более в вопросах, которые вы сами не считаете критическими, стоит сделать так, как попросили.
На мой взгляд тут и спорить особо не о чем. Если не нравится, то, о чём просят, можно отдельно с ним обсудить свою точку зрения. Но не в рамках ревью.
Если это вопрос вы считаете важным, например, какая-то вручную выписанная сортировка вместо стандартной и это просят исправить, то я бы в коментарии в ревью написал бы свою точку зрения о том, почему считаю более правильным испольбзование "ручной" и исправил бы на то, что просят.
Если ревьюер не главный, то я в подобных случаях, когда сам делаю ревью и не считаю замечание принципиальным, сразу же отмечаю свой комент как Resolved/Closed что бы показать, что это просто минорное замечание ("лучшее" на мой взляд имя переменной или, например, использование out var в IDictionary<,>.TryGetValue(…)) которое автор может проигнорировать если сочтёт нужным.
Если ваш ревьюер сделал не так, что в коментах попробуйте выяснить, на сколько это принципиальное изменение с точки срения ревьюера. Если он считает это принципиальным, а вы принципиально с этим не согласны, пускай "главный" и решает. Обычно в ту или иную сторону разработчики способны сами договориться как сделать.
Help will always be given at Hogwarts to those who ask for it.
Re[7]: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
_AB>>А ты оверквоттер? Прояви уважение к собеседникам, отформатируй свои сообщения так, чтобы их читать было удобно. _AB>>К коду ты тоже так относишься, кстати? A_L>Ты своим указывай как что делать
3. Запрещается излишнее цитирование (overquoting). Если вы отвечаете на письмо, цитируйте из него только те отрывки, которые действительно необходимы для понимания, о чём идёт речь. Наличие в сообщении трех цитат подряд или строки "X>Здравствуйте, Ник, Вы писали:" является причиной для бана.
Help will always be given at Hogwarts to those who ask for it.
Re[5]: Как решать конфликтные ситуации при code review?
Здравствуйте, Pzz, Вы писали:
Pzz>>>А что, вышестоящий товарищ более компетентен по определению?
vsb>>Конечно. На то иерархия и выстроена.
Pzz>Это иерархия управления и ответственности, а не технической экспертизы.
Обычно в компании есть иерархия по техничекой экспертизе. И тимлид в её рамках является высшим звеном, разве не так? Джуниор-Мидл-Синьор-Тимлид. В моём понимании — так.
Ну и, раз упоминаем про ответственность, то к ней и повышенные права должны прилагаться.
Здравствуйте, vsb, Вы писали:
Pzz>>Это иерархия управления и ответственности, а не технической экспертизы.
vsb>Обычно в компании есть иерархия по техничекой экспертизе. И тимлид в её рамках является высшим звеном, разве не так? Джуниор-Мидл-Синьор-Тимлид. В моём понимании — так.
Задача тимлида — делегировать ответственность. Но не делать работу за подчиненных. Он не обязан быть самым крутым экспертом, его задача, как тимлида, управлять людьми. Кроме того, в группах с высокой экспертизой (т.е., занимающихся сложной, а не трудоёмкой работой) часто практикуется специализация. Не обязательно, чтобы все понимали во всём.
Ну и наконец, тимлид может быть просто очень занят. Он управляет несколькими людьми и взаимодействует с начальством.
vsb>Ну и, раз упоминаем про ответственность, то к ней и повышенные права должны прилагаться.
Права — да. Но переделывать работу за подчиненного — это управленческое фиаско.
Re: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>Добрый день
A_L>Часто бывают спорные ситуации во время code review. Это либо опыт у людей разный, либо дело вкуса, какие-то "холиварные" темы A_L>Например, писать/не писать комментарии, название переменных, использовать/не использовать var и т.д.. Список можно продолжать до бесконечности A_L>Это те моменты, которые никак качественно не влияют на работу приложения. Т.е. например, это не о том как код используется больше/меньше ресурсов
A_L>Team leader проверяет, вносит свои правки, а у автора кода другое мнение как лучше для проекта
A_L>Как поступать в подобных ситуациях — соглашаться с мнением team leader или продолжать отстаивать свое?
A_L>Обратиться к кому-то "выше" за разъяснениями нет возможности
1) должен быть код стайл и все разногласия обсуждаются, принимается единое решение и заносится туда
2) Писал в разных код стайлах, в целом плевать можно принаравиться под любой.
на шарпах как-то давно был срач
Подход 1:
приватные поля класса как _field, константы UPPER_CASE, приватные методы getSmth, а публичные GetSmth
Подход 2:
в другом проекте баня почему-то упала и приватные поля, стали писать как field константы как CamelCase
на все мои аргументы что когда параметр метода идентичен полю класса, то обязательно приходится писать this — мимо кассы.
то, что в контексте сразу видно приватный метод, тоже мимо кассы и константы туда же.
так же были любители var
и когда в СКВ смотришь var someVar = GetData() то какого типа someVar вообще непонятно.
И там где была власть, был подход №1 и плевать я хотел на код стайлы микрософта.
В джавовом проекте видел интерфейсы с I, что в общем очень удобно, но у джавистов с этого "горит", хотя мне кажется, что удобно,
но в джаве так не принято, плюс jdk написан без префикса I.
Но для большой системы — удобно.
Re[2]: Как решать конфликтные ситуации при code review?
Здравствуйте, diez_p, Вы писали:
_>приватные методы getSmth, а публичные GetSmth
А это зачем?
_>на все мои аргументы что когда параметр метода идентичен полю класса, то обязательно приходится писать this — мимо кассы.
+1
_>то, что в контексте сразу видно приватный метод, тоже мимо кассы
А смысл в этом какой?
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Re: Как решать конфликтные ситуации при code review?
Здравствуйте, Aleksei_Lekomtsev, Вы писали:
A_L>Добрый день
A_L>Часто бывают спорные ситуации во время code review. Это либо опыт у людей разный, либо дело вкуса, какие-то "холиварные" темы A_L>Например, писать/не писать комментарии, название переменных, использовать/не использовать var и т.д.. Список можно продолжать до бесконечности A_L>Это те моменты, которые никак качественно не влияют на работу приложения. Т.е. например, это не о том как код используется больше/меньше ресурсов
A_L>Team leader проверяет, вносит свои правки, а у автора кода другое мнение как лучше для проекта
A_L>Как поступать в подобных ситуациях — соглашаться с мнением team leader или продолжать отстаивать свое?
A_L>Обратиться к кому-то "выше" за разъяснениями нет возможности
Краткосрочно — делать как говорит тимлид.
Долгосрочно:
1. Давить в себе это "мне больше всех надо". Вам с этим проектом (с этим куском кода) детей не рожать.
2. Искать другой проект, другую компанию — в надежде, что там будет не так.
3. Становиться тимлидом. Тимлидство бывает разное — и с перевесом в менеджмент, и с перевесом в программирование.
Здравствуйте, Quebecois, Вы писали:
Q>У меня так было — сидишь, продумываешь ортогональную декомпозицию, делаешь красивые уровни абстракции, а на ревью начинается — тут переименовать, там break вместо return, здесь аргументы поменять местами, и сиди переделывай все по нескольку раз. А что код делает, люди понимать даже не пытаются. Уволился — полегчало. Проект потом закрыли.
Так они наоборот пытаются, но там return вместо break да и именования какие-то странные. Пойди разбери что этот код делает!
Разве так можно? 
