Негативный опыт с code review

Ребята, я готовлю доклад на конференцию о том, как не надо проводить код-ревью и как негативные моменты этого процесса влияют на атмосферу внутри команды.

Ответьте, пожалуйста, на вопросы небольшого опроса, который я подготовил для этой темы — это не займет больше 3 минут: goo.gl/Uaax25

PS: у кого есть твитер, я буду признателен за ретвит ссылки на этот опрос: twitter.com/...​status/530735105212710912...
Mehr anzeigen

👍ПодобаєтьсяСподобалось0
До обраногоВ обраному0
LinkedIn
Дозволені теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter
Дозволені теги: blockquote, a, pre, code, ul, ol, li, b, i, del.
Ctrl + Enter

Проблема, которая вылезает везде, и которая не описана в предложенном вопроснике — это оперативность ревью. Как правило, если на него не смотрят более двух дней, начинает нервировать и задерживает работу. Мой знакомый ушёл с проекта из крупной аутсорсинговой конторы потому, что представитель от заказчика тянул с оценкой неделями, за это время кодовый апстрим успевал существенно измениться, и приходилось переделывать ревью заново. Это может влиять сильно больше, чем собственно критика.

В остальном, из моего опыта (ограниченного только продуктовыми компаниями, где нет механической смеси исполнителей проекта, подобранных просто из тех, что были свободны) — собственно негативная оценка на ревью, если её автор держится в рамках принятой корректности, не даёт проблем. Конечно, в зависимости от местности это может быть от «У меня есть некоторые подозрения, что это место не до конца проработано...» до «Что за херня, переписать заново», но сколь-нибудь адекватный автор не воспримет это как оскорбление.

Посмотри на форумы, как народ в массе своей реагирует на подобные посты.

О каких именно форумах речь? Прошу парочку реальных примеров для уточнения контекста.

Неважно, посмотри на этот. А так любой.

Не смог найти примера. Может, потому, что не хожу по явным помойкам. Но на явных помойках вряд ли будет существенный код:)

Как среагируешь?

Попрошу подробного обоснования. До получения такого обоснования — проигнорирую.

очень негативно воспринимается подавляюшим большинсвом народа.

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

А должны быть исключительно культурные, а не посередине.

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

Ты просто придумал про стиль, основываясь всего-лишь на том, что кто-то посмел уточнить твою точку зрения.

Нет, это говорит о Вашей, а не о моей реакции.

То, как именно Вы это «уточняли».

Ты проигнорируеш, а пять других — обидятся.

Тебе же сказали — «в зависимости от местности». Можно даже уточнить — в зависимости от того, «как тут принято».

Вот лично я люблю, когда говорят прямо. Не «у тебя отлично получилось, но мне кажется, что, возможно, существует более элегантное решение», а прямое «ну это же херня!».

Точно также существуют ранимые личности, которым такая фраза будет прямым уязвлением самомнения. И это тоже стоит учитывать.

А я разве говорю, что все люди толстокожие? В конце концов, ты знаешь свою команду, и знаешь кому что можно говорить. И да, довести человека до увольнения/нервного срыва вполне можно и исключительно вежливо.

Ключевая фраза — «в рамках принятой корректности». Остальное-то указано лишь как пример. Зачем придираться к словам?

Рамки конкретного человека неизвестны.
Если ты с этим человеком достаточно проработал — то все тебе известно, не прибедняйся.

А гнуть матом на незнакомого человека, не только некорректно, но и вредно для здоровья :)

P.S. Нет, правда, у тебя комментарии пишут в стиле «Уважаемый сударь, ваша реализация чудесна! Однако, мне показалось, что количество условий в этом методе слегка чрезмерно. Возможно, для продуктивной работы в дальнейшем, вы соблаговолите изменить этот код?»

Да почти в таком виде
Забавно, но я бы такое воспринял как насмешку.
желательно четко описать, что именно ты поправил бы и почему
Т.е. ты предлагаешь унижать разраба тем, что он не в состоянии придумать сам хорошее решение?
Ты просто предлагаешь свое видение.
И тем самым намекаешь, что ты лучше его.
Кстати, шутки-шутками, но уперание рогами из-за «моя идея лучше, потому что она моя» — я реально видел.
чтобы бить на эмоции — иногда и это надо, но очень очень редко.

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

Вообще-то «это херня» можно сказать не грубя. Плохо. Не подходит. Надо переделать.

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

P.S. И да, не важно какими словами это сказано, но «Это херня, все переделай» — в любом случае крайне плохая рецензия.

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

Такое согласование может сильно затянуться :)

Демократия здесь не работает, есть смысл обсудить на самом высоком уровне техлид и несколько senior-ов и выдать уже готовую процедуру code-review, которой все обязаны следовать, в идеале внедрить в сам билд проекта. Малейшая слабина в принятии решения и все рушится. Т.е. правила формально есть, но фактически на них забивают, при этом осадок у всех остался.

Вы пишете на собственном опыте?

на самом высоком уровне техлид и несколько senior-ов и выдать уже готовую процедуру code-review, которой все обязаны следовать, в идеале внедрить в сам билд проекта.
Может в идеале они сами на самом высоком уровне начинают делать код-ревью согласно принятой процедуре и приучают этим остальных, впоследствии расширяя круг тех, кто делает ревью?..

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

Поделись результатами и анализом, когда соберешь данные. Если об этом еще не просили.

Можно будет послушать на XP Days меньше, чем через месяц: xpdays.com.ua/...ew-antipatterns

Отлично, билеты уже купил. Но все равно, тем, кото принимает участие в опросе было бы интересно узнать результат, думаю.

Код-ревью — однозначно полезная и нужная вещь. Но ее нужно уметь правильно применять. Мне кажется обосновано применение 1-2 опытных ревьювера и на фичи или на джунов.

Сталкивался с ситуацией (не в EPAM), когда вся команда из 5-7-9 человек должна была ревьювить любой чих, даже изменение в проперте или на JSP-ке + сюда добавлялся напряг добавить на каждый коммит +4 теста. Понятное дело что ценность инструмента резко снизилась со всеми выходящими последствиями.

Кратко — не лучше. Тем же миддлам нужна передача опыта, а джунам и подавно. К тому же важно чтобы senior-ы ревьювили код друг друга и были общие командные подходы, а не огораживание. Иначе получится как здесь:
«Сидоров предлагает крупноблочную архитектуру. Петрович настаивает, что все надо строить по старинке, из кирпича, не по-ламерски.»......."Просто 12 этажей Сидорова на 4 метра выше и на 5 метров шире, чем 12 этажей Петровича. Выяснилось, что они строили из разных панелей."

Во-первых senior — более редкий, капризный и ограниченный ресурс. Вы попробуйте за полгода набрать хотя бы 20-30 senior-ов при той же зарплатной политике, если не верите, спросите у рекрутеров.
Во-вторых это глупо стрелять из пушки по воробьям. В армии например есть и обычные и 12,7 пулеметы и гранатометы и 40/100/120/152мм и корабельная 330мм артиллерия, каждая под свою задачу. Для многих задач senior-ы не нужны, достаточно миддлов или джунов.

Тем же миддлам нужна передача опыта
"обмен опытом" звучит красивей)

Я не ошибся, «обмен опытом» может и звучит красивее, но в данной связке этого не происходит, тут именно передача от более опытного к менее.

От вам і негативний досвід code review, коли замість того щоб оцінити ваш внесок,
вам зробили такі прості зауваження:

що за форма з країнами — «америка, німеччина, і два „інших“»??
А каким боком Software Testing Engineer к code review?
Reviewed. Try to fix ASAP.

А каким боком Software Testing Engineer к code review?

Тестировщики также пишут код и участвуют в ревь кода, написанного другими тестировщиками

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

а что можно сделать ревью не на поровшись на проблемы если у вас работают неадекватные упыри?

А зачем вы нанимаете на работу неадекватных упырей?;))) — может в этом проблема?..

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

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

мне это не надо доказывать, докажите менеджерам решившим что самый упоротый аспергер это лучший кандидат делать всем ревью ))))

менеджерам решившим что самый упоротый аспергер это лучший кандидат делать всем ревью ))))
Цитата
А зачем вы нанимаете на работу неадекватных упырей?;))) — может в этом проблема?..
касается менеджеров в первую очередь. Вот зачем их таких нанимали?)
Аспергеры бывают, это не самое страшное. Иногда им лучше просто не мешать — и они сами наваяют вполне.
кое кто говорит в гугле например дико много политики на этом замешано. и ревью оч популярный тул для выяснения отношений
 — совершенно не соответствует моему опыту, механизм code review в Google работает очень хорошо

я слышал и людей очень не довольных
видно кому как с командой свезло

Возможно. Мне свезло с командой трижды.

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

Сергей, ну вы умный, придумайте что-нибудь чтобы сохранить всем адекватность)

Маша — Катя, наборот, стоит развивать в людях некоторую неадекватность, то есть уход с низшего физиологического уровня и заимение некоторых принципов немного выше него.
И, подумайте сами, разве адекватная девушка станет интересоваться высказываниями Саши Грей?;)

разве адекватная девушка станет интересоваться высказываниями Саши Грей?;)

А можна з цього місця детальніше? Про які таки висловлювання йде мова? :) Щось накшталт cs317327.userapi.com/...hVyaIHWXZmg.jpg ?

Не ображайте дівчину, насправді там було щось про фемінизм
www.youtube.com/...h?v=42EIKQP4Mto

Кстати комент выше — прямая иллюстрация насчет неадекватности.
Благодарю.

Вы кажется не заметили, что я отделяю человека от определенного периода ее работы. Я не интересуюсь Сашей Грей. Но вот то, что делает Марина Аня, достойно некоторого внимания. Ее же образ в порно мне неинтересен абсолютно (так же как и ее видео — в котором ее слов практически и нет, благо ее рот занят), но это вопрос вкуса.
Теперь насчет adult video. Адекватные люди вполне интересуются таковым — вопрос в том, что многим они перестает быть интересным. Разве что изредка сходить парой в хороший кинотеатр на хорошее кино. Жовтень к примеру очень хороший был — и Киев (насколько могу судить по отрывочным впесатлениям). Подростки, интересующиеся таким видео, вполне понятны. Но взрослые люди ... разное бывает.
Адекватность же вполне базовая вещь. Она вполне может быть у каждого и ее можно развивать.
Вопрос в том что во многих профессиях полно разных факторов. И где-то прибыль компании важнее чем состояние и целостное развитие работников.

Тю, теж мені, проблема... Замініть пєчєньки ноотропами.

Мне хватает чтения Доу и настольных игр;)

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

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

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

Разные у людей проблемы, я так понял, что ТС читает for beginners.

Ну не знаю, мне приходилось сталкиваться с тем, что ревью может вызывать неприятные чувства у людей. Это может быть связано совсем не с их профессионализмом, или его отсутствием. У человека может быть плохое настроение, черная полоса, а вы и не в курсе. Всяко может быть. При общении в команде оценка чего угодно может привести к тому, что человек вместо рационального взгляда на критику/замечания может проявить (или ощутить) эмоциональную реакцию.

Наверное в этом случае проблема не с ревью?..

Естественно не с ревью. А разве опрос касается непосредственно проблем и техник codereview? Мне казалось тут как раз хотят выяснить человеческий фактор.

Опрос непонятно чего касается. И мало что выясняет конкретного с человеческим фактором.

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

Это обычная реакция человека, когда его код ревьювят в первый раз;)

Мне сложно понять суть некоторых вопросов, не ясно как понимать «Do you feel that the code reviews you do for other people ...» (Кажется ли вам что проводимое вами ревью кода других людей ...?). Я ожидал бы более четкого вопроса. Например «Ваши ревью ...?» (Do your reviews ...? Are your reviews ...?)

Have you had your code reviewed by another person? => вопрос частично дублирует «Which role have you had in code review activities?» с первого шага :)

Да, спасибо, за обратную связь.

Некоторые вопросы действительно длинные, и их надо переформулировать.

видаляйте одразу :)))
що за форма з країнами — «америка, німеччина, і два „інших“»??
україну в студію :))

There is an “other” item there ))))

в таких випадках пропоную опитувальник постити на other сайтах, а не на доу :)))

А він і запощений на Google Docs )

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