Comments 61
Я считаю, что профессионалы должны быть готовы открыто обсуждать друг с другом результаты своей работы. В том числе во время обзора кода. Без грубости, спокойно, аргументированно, но открыто и честно.
Если код можно написать более эффективно и более идиоматично, то это нужно сделать. Если есть более эффективный алгоритм, то его нужно использовать. Либо аргументировать, почему в данной конкретной ситуации этого делать не нужно.
Мы технические специалисты, а не "художники с тонкой и хрупкой душевной организацией".
> Мы технические специалисты, а не «художники с тонкой и хрупкой душевной организацией».
Посмеялся. Это как «я женщина, а не посудомойка».
У нас профессионал начинается с 5+ лет опыта. А до этого нужно еще дожить. Это в США стажер проходит 3-5 часов собеседования, а у нас всё проще. Если ты не гуру проекта, языка, рефакторинга или технологии, то ревьюить тебе бессмысленно. Так что я бы с тобой очень сильно поспорил. У нас до 5+ лет, как раз таки «художники с тонкой и хрупкой душевной организацией». Я лично вообще не видел человека, который может ревьюить с опытом меньше 3-5 лет на фултайме или меньше синьера. Большинство начинающих программистов даже читать про рефакторинг не начинали.
Посмеялся. Это как «я женщина, а не посудомойка».
У нас профессионал начинается с 5+ лет опыта. А до этого нужно еще дожить. Это в США стажер проходит 3-5 часов собеседования, а у нас всё проще. Если ты не гуру проекта, языка, рефакторинга или технологии, то ревьюить тебе бессмысленно. Так что я бы с тобой очень сильно поспорил. У нас до 5+ лет, как раз таки «художники с тонкой и хрупкой душевной организацией». Я лично вообще не видел человека, который может ревьюить с опытом меньше 3-5 лет на фултайме или меньше синьера. Большинство начинающих программистов даже читать про рефакторинг не начинали.
тем более надо учиться обосновывать свой выбор — пока поясняешь либо сам поймёшь что был прав или не прав, либо же коллеги покажут что можно доработать если в этом есть необходимость.
Грустно, но очень правда…
Ревью это еще и обучение. начинающий и думающий разработчик может подхватить хорошие практики, которые использовал сеньор. А иногда и сеньору есть чему поучиться у начинающего.
У нас до 5+ лет, как раз таки «художники с тонкой и хрупкой душевной организацией»
По факту — технические специалисты низкой квалификации. Же.
Дело не в тонкой душевной организации.
Любой человек может выйти из себя, если его постоянно критиковать, хоть и за дело. Вместо того, чтобы учиться, он будет лишь огрызаться, и в итоге возненавидит вас и уйдет из проекта. Да, если вы все это время были правы, а он — нет, то это еще более менее можно назвать положительным исходом. И это при том, что вы создали токсичную атмосферу и настроили против себя возможно и других членов команды.
А что если вы не всегда были правы? Что если временами он был прав? Что если вы придирались абсолютно по пустякам, просто потому что привыкли придираться к конкретно этому члену команду? Наконец, что если вы рядовой член команды, как и постоянно вами критикуемый, и в общем-то не несете определенной ценности в работе — просто кодер? Мне кажется, в данном случае, вы только ухудшаете атмосферу на работе и в коллективе. И я бы как начальник, как минимум, провел бы воспитательную беседу с обоими.
В конце концов, ничего сложно нет в том, чтобы быть вежливым коллегой и тактично рецензировать работу. Пальцы от этого не отвалятся, а язык не отсохнет. Но при этом, в коллективе будут дружеские отношения, да и вашей же репутации и авторитету это пойдет только на пользу.
Любой человек может выйти из себя, если его постоянно критиковать, хоть и за дело. Вместо того, чтобы учиться, он будет лишь огрызаться, и в итоге возненавидит вас и уйдет из проекта. Да, если вы все это время были правы, а он — нет, то это еще более менее можно назвать положительным исходом. И это при том, что вы создали токсичную атмосферу и настроили против себя возможно и других членов команды.
А что если вы не всегда были правы? Что если временами он был прав? Что если вы придирались абсолютно по пустякам, просто потому что привыкли придираться к конкретно этому члену команду? Наконец, что если вы рядовой член команды, как и постоянно вами критикуемый, и в общем-то не несете определенной ценности в работе — просто кодер? Мне кажется, в данном случае, вы только ухудшаете атмосферу на работе и в коллективе. И я бы как начальник, как минимум, провел бы воспитательную беседу с обоими.
В конце концов, ничего сложно нет в том, чтобы быть вежливым коллегой и тактично рецензировать работу. Пальцы от этого не отвалятся, а язык не отсохнет. Но при этом, в коллективе будут дружеские отношения, да и вашей же репутации и авторитету это пойдет только на пользу.
Что если временами он был прав?
Честно сказать "да, я был не прав. Спасибо за подсказку" разве нельзя?
Если придираешься по мелочам, то это увидит вся команда и вполне нормально будет, если тебе обратят на это внимание, т.к. ты сам мог и не понимать этого
во-первых, не все могут сказать «я был не прав». во-вторых, то что я говорю, как раз и избавит от таких неприятных бесед с командой. По-моему, проще изначально быть тактичным.
В общем, в двух словах: ты ставишь reject на пулл-реквесте, и он в мастер уже не попадет, так какой смысл при этом быть еще резким и грубым?
В общем, в двух словах: ты ставишь reject на пулл-реквесте, и он в мастер уже не попадет, так какой смысл при этом быть еще резким и грубым?
Как-то раз я не мог пройти ревью просто с фразой «зачем ты сделал ПО-СВОЕМУ??? Все равно в итоге будет по-моему или я не позволю залить этот код в апстрим, так зачем тратить время?». Причем самое обидное, что я был готов сделать так, как просят, если бы мне внятно объяснили задачу, а не сказали «сделай так-то так-то», а потом выяснилось, что делать надо совсем иначе. Доходило до смешного — практически любые стили использующие calc приходилось переделывать на JS, потому что «это нечитаемый декларативный стиль, а вот скрипт отладил, и по шагам все понятно, что где происходит».
И да, фразу «я был не прав» от человека не слышал ни разу. Даже когда мне предлагалось делать в REST API запуск сервиса по get-запросу. «Зато в браузере удобно набирать, а твой пост только через утлиту отправить можно»©.
И да, фразу «я был не прав» от человека не слышал ни разу. Даже когда мне предлагалось делать в REST API запуск сервиса по get-запросу. «Зато в браузере удобно набирать, а твой пост только через утлиту отправить можно»©.
Мы технические специалисты, а не "художники с тонкой и хрупкой душевной организацией".
Любой программист — человек. А любой человек может быть демотивирован бездарно-построенным процессом управления или никакой культурой.
Относиться к сложным системам типа "человек" как к простым биомеханизмам типа "бактерия", в которых не надо особо разбираться — сродни забиванию гвоздей микроскопами.
1. Люди разные — не все способны принимать критику, признавать свои ошибки и т.д., не говоря уже о том, что значит слово профессионал и какой процент таких людей в команде :)
2. Мне кажется упущен главный момент «в распределенной команде», где люди зачастую и не видели друг друга в лицо и пулл реквесты — это и есть 90% общения. А если учесть, что многие большие компании имеют сотрудников не то что в разных офисах или городах, а на разных континентах. В таких случаях умение правильно комуницировать проблему — это путь к здоровой атмосфере на проекте.
Не стоит забывать, что если человек (с которым вы лично не знакомы) начинает учить жизни — мы начинаем считатать егомудаком нехорошим человеком — так работать защитный механизм на подсознании большинства людей.
Более того, никто не отменял стереотипы. Как бы вы отнеслись если бы некий Камлеш Кумар Кришна начал бы оставлять каменты типа «все не то и все не так».
Так что обеими руками за корректные каменты описанные автором.
2. Мне кажется упущен главный момент «в распределенной команде», где люди зачастую и не видели друг друга в лицо и пулл реквесты — это и есть 90% общения. А если учесть, что многие большие компании имеют сотрудников не то что в разных офисах или городах, а на разных континентах. В таких случаях умение правильно комуницировать проблему — это путь к здоровой атмосфере на проекте.
Не стоит забывать, что если человек (с которым вы лично не знакомы) начинает учить жизни — мы начинаем считатать его
Более того, никто не отменял стереотипы. Как бы вы отнеслись если бы некий Камлеш Кумар Кришна начал бы оставлять каменты типа «все не то и все не так».
Так что обеими руками за корректные каменты описанные автором.
Насчет пункта 2 не совсем согласен: когда критика приходит по адресу, например лично я принимаю её с какой-никакой благодарностью, ибо я обучаюсь таким образом, в худшем случае имеется злость на себя из-за наступания на одни и те же грабли в который раз. Но когда вполне нормальный код комментируется одним WTF и блокированием мерджа, то тогда такие мысли возникают, да. Но тут уже вопрос как мне кажется не в защитных механизмах, а в том, что это вполне нормальная оценка человека, который подобный образом относится к ревью.
За очевидные баги, найденные на этапе ревью, нужно унижать, как я считаю (кроме ревью в статусе WIP)
UFO just landed and posted this here
вопрос в степени очевидности при ревью. Попадаются такие — что прям берёшь из коммита — запускаешь, а оно сваливается на чемн-ть типа синтакс еррор. или при простых кейсах неопределённая переменная или ещё что в таком роде. ну и тестов, разумеется, на новый код нет.
Впрочем, как и подтверждается из других статей про хабр, главное — не высказывать свое мнение, которое не соответствует серой биомассе, а просто поддакивать, мол, какая классная статья.
Впрочем, как и подтверждается из других статей про хабр, главное — не высказывать свое мнение, которое не соответствует серой биомассе, а просто поддакивать, мол, какая классная статья.
Вот еще хорошая статья про кодревью: https://blog.alphasmanifesto.com/2016/11/17/how-to-perform-a-good-code-review/
Кстати, а Gerrit для ревью использовать вы пробовали?
Кстати, а Gerrit для ревью использовать вы пробовали?
А вот о гораздо бОльшей проблеме с ревью, которая называется «Сделаю по-быстрому, все баги ревьюеры найдут» — ни слова.
Есть такое. Иногда можно делать «пост-ревью» уже замёрженного кода, что-то да найдется.
Очевидно, что код ревью не позволяет найти все баги. Получит завелнные тесты или билд после коммита, начнет думать.
Если у таких разработчиков убрать код ревью, они «сделают по-быстрому, все баги тестировщики найдут». То есть проблема не в код ревью, а в самих людях.
Если у таких разработчиков убрать код ревью, они «сделают по-быстрому, все баги тестировщики найдут». То есть проблема не в код ревью, а в самих людях.
Ой ну прям не программисты, а кисейные барышни на прогулке.
Конечно, нельзя оскорблять коллег, конечно, нельзя показывать свое (возможное) превосходство, но не надо сидеть часами подыскивать слова, только чтобы никого не задеть. Eсли человек, к примеру, из раза в раз делает плохо, сказать ему "Как я уже говорил" или "Мы это уже обсуждали" — абсолютно нормально.
Все также зависит от организации: если это open-source, то да, можно и поаккуратней, чтоб не спугнуть новичка, но если это человек, который получает за свою работу деньги и он лажает — ему надо об этом сказать прямо, а не скакать козликом вокруг да около
Если челоек нарушил принятые гайдлайны оформления кода, то на вопрос "Почему ты так сделал?" он ответит "А что? Что не так?" и все равно вашей следующей репликой будет "Ты написал не по гайдлайнам". Так зачем эта лишняя итерация?
UFO just landed and posted this here
Eсли человек, к примеру, из раза в раз делает плохо, сказать ему «Как я уже говорил» или «Мы это уже обсуждали» — абсолютно нормально.
… а потом выяснится, что «плохо» — оно не плохо, а просто субъективный взгляд делавшего ревью. Который никто (по данному конкретному узкому вопросу) не принимает в расчет.
Ой ну прям не программисты, а кисейные барышни на прогулке.
Конечно, нельзя оскорблять коллег, конечно, нельзя показывать свое (возможное) превосходство, но не надо сидеть часами подыскивать слова, только чтобы никого не задеть. Eсли человек, к примеру, из раза в раз делает плохо, сказать ему "Как я уже говорил" или "Мы это уже обсуждали" — абсолютно нормально.
Все также зависит от организации: если это open-source, то да, можно и поаккуратней, чтоб не спугнуть новичка, но если это человек, который получает за свою работу деньги и он лажает — ему надо об этом сказать прямо, а не скакать козликом вокруг да около
Если человек нарушил принятые гайдлайны оформления кода, то на вопрос "Почему ты так сделал?" он ответит "А что? Что не так?" и все равно вашей следующей репликой будет "Ты написал не по гайдлайнам". Так зачем эта лишняя итерация?
UFO just landed and posted this here
Проблема формальных ревью имхо в их крайней неэффективности.
1) по сути они склоняют всех к мультитаскингу — ибо сделал изменения — жди вопросов/предложений, а чтобы время не пропадало займись чем то еще, и так переключайся туда-сюда по кругу. Мультитаскинг надо избегать ибо он жрет эффективность и концентрацию.
2) зажимают все общение в рамки письменного изложения. Письменно прийти к решению/пониманию в разы сложнее/дольше чем вербально. Не все умеют излагать мысли на бумаге, не все умеют понимать абстрактные идеи изложенные письменно (кому то нужна картинка например или какая то жизненная аналогия).
3) люди в большинстве своем не умеют передавать эмоции письменно (не все родились поэтами), соответсвенно могут портится отношения на ровном месте (фокус статьи я так понимаю на этом), ибо нет эмоционального фидбэка и корректировки.
Так что если можете избегать формального ревью — делайте это.
Парное программирование, работа всей команды над одним модулем/компонентом (а не каждый в своем огородике), правило "максимально просто плюс есть тест — ревью пройден заочно", кросфункциональные команды (таск форс), и многое другое позволяет достигнуть всех плюсов формальных ревью не сжигая на это времени/нервов/ресурсов.
а я бы не отказывался. В процессе формального ревью порой отлавливаются баги, фиксание которых может занять больше времени, чем ожидание когда твой пулл-реквест «заапрувуят». Также, ревью помогает не допустить добавление в проект спорных дизайнерских решений, исправление (рефакторинг) которых, может занять очень много времени. Ну и наконец, формальные ревью полезны тем, что помогают команде ознакомиться с тем, что делают остальные члены и подать голос, в случае если остальные с чем-то несогласны. Не получится так, что приходишь в среду на работу, а в master кто-то наговнокодил и после этого полдня команда разбирается с последствиями.
UFO just landed and posted this here
Подождал да…
Зависит от процессов в которых вы работаете или на какие хотите выйти.
В лине (lean) «подождал» это уже waste, пусто потраченное время.
Возможно для процессов поддержки продукта, багфикса, ревью более-менее рабочее решение как девелоперов хоть как-то мотивировать читать чужой код и делится опытом и наработками.
Но когда команда уже работает над одним и тем же модулем (а не в отдельных огородиках), в тесном контакте, работает инкрементально, ревью становятся 3ей ногой которая мало того что требует ресурсов и времени на поддержку так еще и может захоти пойти в другую сторону. Получается такое ревью ради ревью, которое люди по старой памяти пытаются притянуть куда можно.
При работе удаленно парное последовательное программирование на моем опыте работает просто огненно: один девелопер заканчивает работу над модулем в своем часовом поясе/графике, отписывает что сделал и что получилось что еще нет, второй подхватывает доделывает/исправляет, и передает работу опять первому. Очень эффективно работает.
В том же ревью по умолчанию предполагается что если написали коментарий, то автор должен внести изменения. В случае с удаленной работой/поясами задержка минимум день или два, плюс добавим пару дней если кто-то чего-то не понял. К чему этот цирк и почему коментатору взять и не исправить самому? Опять же на моей практике это еще более эффективно чем писать простыни текста в ревью, взял и исправил.
Далее работа инкрементально… Представьте идет работа над новым продуктом, в идеологии агиле/лин: нет железного дизайна, есть цели которые достигают инкрементально, дизайн строится по ходу понимания проблемы все лутше и лутше, на каждой итерации рефакторя или переделывая первоначальные концепты. И вот, на первой или второй итерации вся команда начинает ревьювить и вылизывать первоначальный пруф-концепт, который наверняка в 4ой или 5ой итерации полностьт перепишут. Какой смысл это ревьювить и вылизывать? Никакого.
Надо сказать что я больше пишу про ревью внутри команды. Команда это 2-5 человек, работающая тесно над одной проблемой, в практически полной свободе. Бывает если надо внести изменения в какую-то изолированную организацию/группу ревью может и понадобится (или пулл реквест, смотря что они используют), но обычно это костыли и команда уже не кросс-функциональная.
Зависит от процессов в которых вы работаете или на какие хотите выйти.
В лине (lean) «подождал» это уже waste, пусто потраченное время.
Возможно для процессов поддержки продукта, багфикса, ревью более-менее рабочее решение как девелоперов хоть как-то мотивировать читать чужой код и делится опытом и наработками.
Но когда команда уже работает над одним и тем же модулем (а не в отдельных огородиках), в тесном контакте, работает инкрементально, ревью становятся 3ей ногой которая мало того что требует ресурсов и времени на поддержку так еще и может захоти пойти в другую сторону. Получается такое ревью ради ревью, которое люди по старой памяти пытаются притянуть куда можно.
При работе удаленно парное последовательное программирование на моем опыте работает просто огненно: один девелопер заканчивает работу над модулем в своем часовом поясе/графике, отписывает что сделал и что получилось что еще нет, второй подхватывает доделывает/исправляет, и передает работу опять первому. Очень эффективно работает.
В том же ревью по умолчанию предполагается что если написали коментарий, то автор должен внести изменения. В случае с удаленной работой/поясами задержка минимум день или два, плюс добавим пару дней если кто-то чего-то не понял. К чему этот цирк и почему коментатору взять и не исправить самому? Опять же на моей практике это еще более эффективно чем писать простыни текста в ревью, взял и исправил.
Далее работа инкрементально… Представьте идет работа над новым продуктом, в идеологии агиле/лин: нет железного дизайна, есть цели которые достигают инкрементально, дизайн строится по ходу понимания проблемы все лутше и лутше, на каждой итерации рефакторя или переделывая первоначальные концепты. И вот, на первой или второй итерации вся команда начинает ревьювить и вылизывать первоначальный пруф-концепт, который наверняка в 4ой или 5ой итерации полностьт перепишут. Какой смысл это ревьювить и вылизывать? Никакого.
Надо сказать что я больше пишу про ревью внутри команды. Команда это 2-5 человек, работающая тесно над одной проблемой, в практически полной свободе. Бывает если надо внести изменения в какую-то изолированную организацию/группу ревью может и понадобится (или пулл реквест, смотря что они используют), но обычно это костыли и команда уже не кросс-функциональная.
UFO just landed and posted this here
И опять вы перекладываете с больной головы на здоровую.
Мы просто в разных подходах живем.
Подождал — это, очевидно, значит переключился на другую задачу
Меня такое не устраивает так как приводит к мультитаскингу.
Вы — внезапно! — описали процесс ревью. Только исправляет не автор, а разработчик по очереди.
Так я не против ревью по смыслу. Я против формальных ревью когда людей суют в эти тулзы где можно комментировать и оставлять замечания. Часто начинается бред, статьи которые с ним борятся, хотя прямое общение отфильтровывает многие проблемы. Это так же как старожилы агиле предлагают не использовать онлайн спринт-борды, а использовать обычные физические доски в оффисе и прямое общение — такой формат позволяет избежать множества проблем и общатся эффективнее.
К парному программированию это не имеет никакого отношения. Просто работа по сменам.
Кому как. Для многих и работа «по сменам» это когда у кажлого модуля свой владелец и никто туда не лезет без спроса.
То неловкое чувство, когда не читал публикацию…
Слишком длинно но основные моменты надеюсь уловил.
Вы написали, что удалённая работа менее эффективна, но это не проблема ревью, а процесса работы.
Вообще на своем опыте я постоянно наблюдал ситуации когда люди сидящие в одном оффисе днями спорят в ревью вместо того чтобы встать подойти или какой мит устроить. Удаленная работа всего-лишь обнажает такие вещи и выявляет их неэффективность по сути, так сказать внутренний конфликт идеи.
А если вообще решение неправильно? Причины разные: неправильное понимание задачи, незнание каких-то нюансов и т.п. Если учитель всё время будет исправлять за учеником, то он ничему не научится.
Ну человеку тут же укажут что решение неправильное. По хорошему (агиле-лин процесс) все эти временные изменения должны тут-же собиратся (прямо на коммит), прогонятся юнит тесты и какая-то полуавтоматика, и паралельно роль QA уже тестировать и не ждать от девелоперов «мы закончили». Фидбэк должен быть мгновенным, если что-то сломано об этом человек узнает очень быстро.
Я не думаю что учитель/ученик подходит здесь. В школе ученик сдал задание и забыл о нем навсегда, естественно если учитель исправит это будет в пустоту (фидбэка нет). В случае с рабочей командой же код никуда не пропадает и результат кода видят все (продукт/приложение).
В свою очередь если можно сделать несколькими способами одно и тоже, и кто-то считает что одно правильное другое нет, то тут просто у команды должны быть четкие ориентиры что важно а что нет. Важно закончить в срок и отдать рабочее решение или провалить срок но вылизать кусок кода который имеет минимальный эффект для продукта и клиента? При ясных целях и ориентирах этот бред быстро заканчивается. По существу если ориентирвы выставленны во время итерации правильная команда сначала сделает чтобы все заработало, и уже в оставшееся время в спринте будет тратить на улутшения и фиксы. Если же кто-то кинется писать идеально сразу, или начнется формальное блокирующее ревью до QA, это все просто помножит риски тупо провалить сроки и потратить время в пустую.
Нет ничего более постоянного чем временное. Послезавтра планы поменялись, появились другие задачи и код, «который конечно перепишут», остаётся на года. Вообще-то, вы забываете, сам процесс предполагает, что код пишется максимального качества.
Максимальное качество можно достигнуть разными путями. Можно сразу пытатся все сделать идеально и рисковать сроками и возможностью начать улутшать то что завтра выкинут, а можно улутшать итеративно, минималистично, и то что 100% нужно здесь и сейчас. Я предпочитаю второй подход.
P. S. Вы так и не ответили про работу все командой над одним модулем. Вот вам на новой итерации надо написать модуль. Как вы это сделаете всей командой? Не в три смены, в одном или близком часовых поясах.
Не совсем правильно. Сама постановка вопроса «на новой итерации надо написать модуль» предполагает готовый дизайн, какой-то план по которому идут, т.е. попахивает вотерфолом. Такой постановки вопроса не должно быть вообще. Постановка может быть «нам надо сделать фичу X в продукте Y». Команда делает быстрый ресерч — какие изменения и в каком модуле должны быть сделаны, что протестировано, какие тесты написаны. Соотвественно один может начать писать юнит тесты, QA роль может начать готовить тест кейсы и поднимать инфу на том как воспроизвести окружение, другой девелопер начать писать концепт каких-то изменений в модуле, кто то может начать писать новый модуль (если надо). Часто бывает что надо даже разобраться с каким-то новых low-level API для нового функционала и обьяснить команде его секреты — вот вам и задача на день. Проходит день и команда лутше понимает что надо делать дальше — синхронизация — правим дальше, вместе.
Формальное ревью с блокировкой тут совершенно не в тему, имхо, оно может потянуть команду в старые процессы и весь агиле закончится.
и паралельно роль QA уже тестировать и не ждать от девелоперов «мы закончили»
Замечательная идея… я прямо вижу, как разработчики вносят сегодня изменения в ту часть, которая была протестирована QA вчера. В результате либо QA не отловит проблему, либо будет делать одну работу дважды.
Постановка может быть «нам надо сделать фичу X в продукте Y». Команда делает быстрый ресерч — какие изменения и в каком модуле должны быть сделаны, что протестировано, какие тесты написаны.
Круто, если у вас есть свободная команда под этот проект. У меня на работе, например, в моей команде, которая занимается back-endом, 4 человека, и у каждого своя зона ответственности. Собрать всех вместе, чтобы пилить одну фичу? Не в этой жизни, это тупо расходование ресурсов попусту, потому что 3/4 задач вообще неясны и их постановка непонятна 3/4 команды. И только 1/4 понятна всем.
Если «огородик» — это проблема, значит есть проблемы с интерфейсами и API, а то и ваще нездоровый агиле применяется: «мы слоны, мы сделаем все как просили»
Не все умеют излагать мысли на бумаге
Программирование по сути — изложение своих мыслей на бумаге в исходном коде.
Относительно все конечно излагают.
Но много ли Вы видели программистов которые пишут код 100% по делу, без лишней «пены», максимально просто и эффективно имплементят какой-то известный алгоритм? Таких единицы. Обычно же делается куча лишних движений, толи в попытке скопировать мастеров, толи потому что всегда так делали и так удобней, или нехватает IQ для выявления взаимосвязей и повторимостей.
Еще меньше тех кто умеет не только написать код, но и изложить архитектуру, видеть дальше чем тот кусок кода что сейчас меняется.
И вот если человек не может написать код четко и ясно, как он напишет обоснование, да еще обычно и не на родном языке?
Но много ли Вы видели программистов которые пишут код 100% по делу, без лишней «пены», максимально просто и эффективно имплементят какой-то известный алгоритм? Таких единицы. Обычно же делается куча лишних движений, толи в попытке скопировать мастеров, толи потому что всегда так делали и так удобней, или нехватает IQ для выявления взаимосвязей и повторимостей.
Еще меньше тех кто умеет не только написать код, но и изложить архитектуру, видеть дальше чем тот кусок кода что сейчас меняется.
И вот если человек не может написать код четко и ясно, как он напишет обоснование, да еще обычно и не на родном языке?
по сути они склоняют всех к мультитаскингу — ибо сделал изменения — жди вопросов/предложений, а чтобы время не пропадало займись чем то еще, и так переключайся туда-сюда по кругу. Мультитаскинг надо избегать ибо он жрет эффективность и концентрацию.
Иногда это неизбежно — команда маленькая, ты не можешь ждать, пока освободится твой коллега, который сделает ревью, а освободится он завтра к вечеру.
2) зажимают все общение в рамки письменного изложения. Письменно прийти к решению/пониманию в разы сложнее/дольше чем вербально. Не все умеют излагать мысли на бумаге, не все умеют понимать абстрактные идеи изложенные письменно (кому то нужна картинка например или какая то жизненная аналогия).
Надо учиться. Это имеет огромный плюс — изменение описано формально так, что сможет понять твой коллега, который не в теме — это значит, что через год, когда ты, или кто-то еще будет работать над этим куском, можно будет открыть комментарии к коммиту и описание проблемы и ее решения в task'е — ticket'е и понять, чтож там было, и почему сделано именно так. Это по сути частичное документирование кода. В противном случае получится так: ты что-то исправил, закоммитил с коротким комментарием вида «исправлениа бага с пустым полем ID», потом объяснил на пальцах коллеге за соседним столом, который будет делать ревью, а через день-два ВСЕ забыли, что это было вообще, не говоря уж о деталях реализации или том, что сам баг вызывался путем манипуляций «два притопа, три прихлопа» и только в полную луну. А так — вот оно — открыл таск — все есть.
Может кто-то поделиться опытом, как создать эту самую атмосферу " вдохновляющий процесс, чтобы все участники ждали его и активно участвовали в нём"? Как помочь товарищам войти во вкус и читать чужие пуллреквесты с удовольствием?
тяга к знаниям и желание сделать проект, в котором ты работаешь, лучше?
как создать эту самую атмосферу
Не следовать советам этой статьи. Людей вдохновляет реализация их идей и признание их профессиональных качеств. Людям нельзя говорить, как делать работу, только что делать. Поэтому в ходе review должны проверяться сугубо формальные требования, чтобы не возникали конфликты. Эти требования должен составить архитектор письменно. Например:
1. сложные не-UI методы должны быть покрыты unit-тестами
2. поля таблиц БД должны быть документированы
и т.д.
Звучит как свобода — свобода делать как хочешь, плюс минимальные рамки для достижения каких-то вторичных организационных целей. Работает замечательно с замечательными людьми на моем опыте.
«людям нельзя говорить, как делать работу,»
Абсолютно не соглаесен. Если ты читаешь чужой код и видишь, как в каком-либо моменте можно сделать лучше, чем есть, обязательно стоит это предложить. Если автор кода аргументированно с тобой не согласен, он объяснит, почему он сделал именно так. В том числе и в этом есть смысле ревью.
Абсолютно не соглаесен. Если ты читаешь чужой код и видишь, как в каком-либо моменте можно сделать лучше, чем есть, обязательно стоит это предложить. Если автор кода аргументированно с тобой не согласен, он объяснит, почему он сделал именно так. В том числе и в этом есть смысле ревью.
Снобов и не умеющих общаться корректно по форме — за дверь, можно немножко отпинать. Там само наладится. Когда человек поймет, что коллеги его реально могут научить интересным трюкам, дать дельные советы, не унижая при этом, сам втянется.
На стенку. Рядом с «Настоящий мужчина должен» и «Настоящая женщина обязана» )))
UFO just landed and posted this here
UFO just landed and posted this here
Вобще правила хорошие, я их все в общении с женой и детьми использую. Особенно хорошо в семейной жизни помогают
пункты «Не переходите на личности», «Избегайте фраз наподобие: „Совершенно неправильно“».
Еще годный совет — «Не используйте нетерпеливых или пассивно-агрессивных оборотов».
Ну и просто бомба — «Не говорите только для того, чтобы услышать свой голос».
В общем всё это прекрасные советы для улучшения взаимопонимания в семье. Я вам как отец 2 детей
и женатый уже 11 лет заявляю.
p.s. Да, для код ревью это тоже годится. Да и вообще для повышшения эффективности любых межличностных взаимодействий!
пункты «Не переходите на личности», «Избегайте фраз наподобие: „Совершенно неправильно“».
Еще годный совет — «Не используйте нетерпеливых или пассивно-агрессивных оборотов».
Ну и просто бомба — «Не говорите только для того, чтобы услышать свой голос».
В общем всё это прекрасные советы для улучшения взаимопонимания в семье. Я вам как отец 2 детей
и женатый уже 11 лет заявляю.
p.s. Да, для код ревью это тоже годится. Да и вообще для повышшения эффективности любых межличностных взаимодействий!
Лично мое опыт ревью показывает его неэффективность в 90%-никогда ревью не находило баги, зато в 99% случаев — это самоутверждение, конфликт, субъективность, ретроградность. Очень часто, хотя и не всегда- это плохая практика, за маркетовыми словами о которой стоит резкое падение производительности команды. Это ревью гарантирует в 100% случаев. И второй негативный эффект ревью — в целом команда становится более серой, яркие решения, хакерский дух, смарт-решения, дающие преимущества в конкурентной борьбе, исчезают из команды, равно как и их носители. В целом это характерно для всей американской ИТ-культуры, которая стоит на фундаменте маркетинга, пиара, зарабатывания денег на процессе внедрения методик и написания книжек. Я рассматриваю это всё в целом — фейл с многими концепциями ООП, UML, agile, паттернами ООП, ревью, ныне супермодный devops, в этом ряду и ревью процесс — всех их объединяет вышеперечисленное. Пи-ар, маркетинг, отсутствие настоящих, объективных критериев, но зато упор на громкие имена и названия их книжек. Проблема с ревью-процессом носит общий характер, корни болезни в западной ИТ-культуре. Контркультура образовалась там же в виде Open source сообществ. И да, во все времена была коллаборация между программистами, сидение за одним монитором и решение какой-то задачки сообща, но это никогда не вело к конфликтам, потому что мотивация была совершенно отличной. Прошу пардона за столь длинный комментарий, но очень хочется услышать единомышленников, уже пришедших к подобному пониманию :)
Критерии то очень просты — успех продукта у пользователя, большая эффективность в его разработке (потратить не годы а месяцы), и чтобы девелоперам нравился процесс и результаты.
Проблема обычно в том что люди начитаются какой-то мотивационной части методик (маркетинг и пиар )) ), начинают их употреблять не по существу а формально (дейли стэндап — как это круто! чтобы не филонили на работе), и потом обнаруживают что ускорения нет а наоборот, плюс еще и девелоперы демотивированы, и потом неправильный вывод делается что виновата методика или вообще вся индустрия/страна/вселенский заговор. Тот-же агиле очень легко зафейлить, например используя ревью — засунув в ревьюверы не участников команды а кого-то со стороны. На этом агиле в принципе и заканчивается с перечисленными Вами проблемами (самоутверждение, ретроградство и так далее).
А агиле то подразумевает что вся команда сама-себе менеджер. Практически полная свобода действий. Но так как для менеджмента это часто уж слишком круто, и будет «хаос» в их понимании, агиле начинается с менеджерами и прочими намордниками и заканчивается так и не начавшись.
ДевОпс тот-же пошел из Гугла/Амазона и компания насколько я понимаю, от лидеров рынка. Какие еще могут быть критерии? Другое дело что когда начинают копировать не понимая как это работает (не разбираясь в принципах на которых построены уже практики) настает печаль.
Проблема обычно в том что люди начитаются какой-то мотивационной части методик (маркетинг и пиар )) ), начинают их употреблять не по существу а формально (дейли стэндап — как это круто! чтобы не филонили на работе), и потом обнаруживают что ускорения нет а наоборот, плюс еще и девелоперы демотивированы, и потом неправильный вывод делается что виновата методика или вообще вся индустрия/страна/вселенский заговор. Тот-же агиле очень легко зафейлить, например используя ревью — засунув в ревьюверы не участников команды а кого-то со стороны. На этом агиле в принципе и заканчивается с перечисленными Вами проблемами (самоутверждение, ретроградство и так далее).
А агиле то подразумевает что вся команда сама-себе менеджер. Практически полная свобода действий. Но так как для менеджмента это часто уж слишком круто, и будет «хаос» в их понимании, агиле начинается с менеджерами и прочими намордниками и заканчивается так и не начавшись.
ДевОпс тот-же пошел из Гугла/Амазона и компания насколько я понимаю, от лидеров рынка. Какие еще могут быть критерии? Другое дело что когда начинают копировать не понимая как это работает (не разбираясь в принципах на которых построены уже практики) настает печаль.
«Вы не умеете его готовить» :)
см. мой коммент как сделать чтобы не было проблем.
см. мой коммент как сделать чтобы не было проблем.
Слишком много и толсто. Со всех сторон, конечно. Но в реальности всё гораздо проще.
Не грубить и не зарываться в своих умных советах, потому как иногда, именно это и рушит команду/взаимоотношения в команде.
За чек-лист плюс. Жаль что это возможно далеко не во всех реальных проектах.
Не грубить и не зарываться в своих умных советах, потому как иногда, именно это и рушит команду/взаимоотношения в команде.
За чек-лист плюс. Жаль что это возможно далеко не во всех реальных проектах.
Хорошая статья. Согласен практически со всем написанным.
Еще важный момент — пресекать холивары, которые могут зарождаться вокруг какой-нибудь фигни. Как правило для этого достаточно немного доброй воли. Для других случаев у нас в команде действует следующее правило, если ревьювер и автор кода прицнипиальное не согласны в каком-либо моменте(условно спорят больше 5 минут), зовем 3го и делаем так как он говорит.
Еще важный момент — пресекать холивары, которые могут зарождаться вокруг какой-нибудь фигни. Как правило для этого достаточно немного доброй воли. Для других случаев у нас в команде действует следующее правило, если ревьювер и автор кода прицнипиальное не согласны в каком-либо моменте(условно спорят больше 5 минут), зовем 3го и делаем так как он говорит.
Код ревью это как чистка зубов — занимает немного времени каждый день, но позволяет оставлять код в здоровом (поддерживаемом) состоянии. Ошибочные комментарии, именования, понятные только автору, нетривиальные циклы в месте, где достаточно стандартного алгоритма. Примеров масса, иногда даже можно увидеть фактическую ошибку, хотя отлов багов целью ревью не является. Бонусом — обучение команды и приведение кода к единому стилю.
Для гладкого процесса у нас есть несколько разумных принципов:
1. Ревью должны быть маленькими. Во-первых, большое изменение очень сложно читать; во-вторых, автор, пиливший код две недели, с меньшей охотой что-то будет переделывать. Если фича предполагает большой объем, то делать предварительный дизайн ревью решения и промежуточные патч-сеты.
2. Замечание — это не приказание исправить. Это мысль, что читающему что-то непонятно или не нравится, причём желательно эти ситуации явно разделять. Поэтому комментарии типа «почему ты здесь использовал А? » хуже, чем «А работает медленно, используй Б».
3. Если приходится писать второй комментарий на одно замечание, то подойди, обговори голосом. Ситуаций вида:
— «А плохо, потому что Б»
— «нет, А хорошо, потому что не только Б, но и В»
— «нет, ты не учел Г»
быть не должно, потому что они слишком быстро ведут к переходу на личности и озлобленности.
4. Если что-то обговорил голосом, допиши резюме обсуждения в комментарий, другим тоже может быть интересно.
5. Если три человека из команды просят переделать одно место, а ты не можешь объяснить, почему твоё решение лучше, переделай как просят.
И максимум того, что можно вынести на автоматическую проверку, надо выносить на автоматическую проверку — наличие интернализации и док блоков (увы, не содержание), проверка на дублирование кода, оформление комит мессаджа по стандарту, прогон статического анализатора, выполнение тестов. На дженкинс разработчику сложно обижаться, даже если он ставит -2 на ревью.
Для гладкого процесса у нас есть несколько разумных принципов:
1. Ревью должны быть маленькими. Во-первых, большое изменение очень сложно читать; во-вторых, автор, пиливший код две недели, с меньшей охотой что-то будет переделывать. Если фича предполагает большой объем, то делать предварительный дизайн ревью решения и промежуточные патч-сеты.
2. Замечание — это не приказание исправить. Это мысль, что читающему что-то непонятно или не нравится, причём желательно эти ситуации явно разделять. Поэтому комментарии типа «почему ты здесь использовал А? » хуже, чем «А работает медленно, используй Б».
3. Если приходится писать второй комментарий на одно замечание, то подойди, обговори голосом. Ситуаций вида:
— «А плохо, потому что Б»
— «нет, А хорошо, потому что не только Б, но и В»
— «нет, ты не учел Г»
быть не должно, потому что они слишком быстро ведут к переходу на личности и озлобленности.
4. Если что-то обговорил голосом, допиши резюме обсуждения в комментарий, другим тоже может быть интересно.
5. Если три человека из команды просят переделать одно место, а ты не можешь объяснить, почему твоё решение лучше, переделай как просят.
И максимум того, что можно вынести на автоматическую проверку, надо выносить на автоматическую проверку — наличие интернализации и док блоков (увы, не содержание), проверка на дублирование кода, оформление комит мессаджа по стандарту, прогон статического анализатора, выполнение тестов. На дженкинс разработчику сложно обижаться, даже если он ставит -2 на ревью.
Sign up to leave a comment.
Ревью кода в распределенной команде