5 советов по проведению хорошего обзора кода

[habralan]

пока я не перевёл «обзоры кода» на английский, я не понял, о чём вообще идет речь… как-то это звучит… подозриительно :)

[andreycha]

Считайте это прихотью переводчика :). Я не очень люблю сплошное употребление англицизмов, и считаю, что если возможно обойтись русским словом — нужно использовать именно его. А тем более «обзор» — прекрасное слово и даже выдумывать ничего не нужно. «Ревью» все же не по-русски звучит, а «ревизия» у меня прочно ассоциируется с системами контроля версий.

[andreycha]

Хотя вот Википедия подсказывает, что есть еще слово «рецензия».

[VasilioRuzanni]

«Рецензирование кода» это обычно называют по-русски.

[truezanuda]

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

[VasilioRuzanni]

Ну оно так называется, что тут поделаешь :) К тому же, рецензирование и написание рецензии — не одно и то же, а зависит от контекста.

[ItGold]

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

[Nashev]

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

[stasikos]

Не хватает еще статьи «что такое обзор кода» и «как проводить обзор кода». :) Было бы интересно.

[andreycha]

Посмотрите Википедию:
ru.wikipedia.org/wiki/Рецензия_кода
Статью в этом же блоге:
habrahabr.ru/blogs/code_review/88075/

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

[stasikos]

Я уж было подумал что это разные вещи. Все таки ревизии и рецензии кода это поиск именно ошибок, а вы говорите что нужно проводить обзор кода не с целью их поиска.

[smind]

> 1. Обзоры кода НЕ ДОЛЖНЫ проводиться с целью поиска ошибок.
не понял что то я этого… ведь я произвожу ревизию кода в основном чтобы увидеть потенциальные ляпы (ошибки) которые пропустил выложивший…

[andreycha]

Думаю, автор статьи намекает, что сейчас, когда есть автоматические тесты, акценты немного сместились в другую сторону.

[d0z]

Думаю, что вы не видели их в действии, ну или не знаете что результат далек по качеству и от просмотра кода 3-4 профессионалами.

[andreycha]

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

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

[d0z]

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

[truezanuda]

Ох, вот бы мне 3-4 профессионала, чтоб мой код просматривали по вечерам, пока я до дома еду. :)

[alexs0ff]

Для целей поиска ошибок, нужно использовать юнит-тестирование.

[smind]

ок, я все понимаю, но как написать тест для «в форме при введенной маске поиска „*\1“ происходит лишнее экранирование \1», адекватно ли время написания подобного теста с патчем в 1 строку…

[burivuh]

Для поиска ошибок нужно использовать системный подход к тестированию. Юнит-тесты не панацея. И обзоры кода не панацея. Нужно использовать различные методики вместе, тогда можно добиться лучших результатов.

[smind]

У нас в команде принято производить обзор нового кода перед его влитием, без ревизии 2-х разработчиков нельзя производить влитие кода в основной ствол.

[andreycha]

Вот это круто.
Как бы еще людей заставить ответственно подходить к делу, а не по принципу глянули одну минуту, «Да, нормально, заливай»?

[icoder]

Как звучит! «влитие кода в основной ствол»

[timurv]

Занимаюсь темой code review тоже, ищу инструменты для автоматизации.

Пока на выбор:
* howsmycode.com/
* www.reviewboard.org/

Было бы здорово, если бы кто нибудь посоветовал работающую схему.

[alexs0ff]

Code collaborator smartbear.com/codecollab.php
Не видел ничего лучше.

[timurv]

Он действительно на столько хорош, что можно его купить и потратить время на интеграцию с git?
Из коробки на сколько я понял, он имеет только консольный клиент, работающий с git.

[alexs0ff]

Да хорош, после reviewboard смотрится как небо и земля.

[smind]

не понимаю как обзор кода можно автоматизировать… какой именно этап имеется ввиду?

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

[alexs0ff]

Автоматизация — это введение инструментов для облегчения code review. По идее, в самом простом варианте можно перед комитом подходить к разработчику и смотреть в монитере его изменения.

[timurv]

Как раз ищется инструмент, чтобы просматривать коммиты, комментировать коммиты построчно, ставить подпись.

[conf]

Смотрите мой перевод и комменты к нему, там про инструменты тоже есть.

[smind]

я пользуюсь mc для просмотра кода и изменений в нем
вот как тут на картинке funkyimg.com/u2/310/066/ydiff_PNG.png

[pento]

Это называется «анализ кода».

> 1. Обзоры кода улучшают качество кода: одна голово хорошо, а две — лучше.
> 3. Обзоры кода помогают узнавать лучшие практики от других разработчиков.

[StrangeAttractor]

> 1. Обзоры кода НЕ ДОЛЖНЫ проводиться с целью поиска ошибок.
> 2. Обзоры кода НЕ ДОЛЖНЫ проводиться с целью проверки соблюдения стандартов кодирования.
> 10 лет назад эти два пункта имели бы смысл в обзорах кода. Однако сейчас вы должны использовать
> автоматические средства тестирования и инструменты, следящие за оформлением кода.

Можно примеры инструментов, следящих за оформлением кода?

[andreycha]

ReSharper, StyleCop. Во втором, насколько я знаю, больше база «правил».

[andreycha]

Так же есть плагин, который скрещивает эти два инструмента:

stylecopforresharper.codeplex.com/

[d0z]

позволю добавить выжимку из правил компании в которой я работаю:
— сама инспекция кода должна производится до митинга. На митинге допускается только озвучивание и обсуждение ранее выявленых недостатков.
— митинг должен длится не более 2-х часов с обязательным перерывом.(причина интуитивно понятна)
— если объем кода велик и не укладывается в одину инпекцию и его можно разбить на составляющие компоненты, то нужно это сделать и обсуждать их отдельно.
— список лиц ответсвенных присутсвующих на инспекции:
1. разработчик (автор)
2. модератор — человек которы контролирует процесс обзора и выносит решения
3. 3-4 инстпектора — люди которые предварительно проинспектировали код
4. человек который ведет учет(чаще всего это автор).
— ни в коем случае код не правится на инспекции — это самая грубая ошибка.
— если инспектора не подготовились то модератор имеет полное право отметить инспекцию (и это правильно потому что просмотр кода на самом миитнге это лишняя трата времени и сил)
— количество человек на инспекции лучше ограничить 5-7 людьми.
— лог инспеции должен быть в общем доступе(своеобразынй обмен опытом)

[andreycha]

Спасибо, интересно.

Правильно ли я понял, что вы делаете инспекции довольно объемных участков кода (например, раз в несколько дней или после завершения куска функциональности)?

[d0z]

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

[burivuh]

Очень похоже на описание формальных инспекций у Макконнела в «Совершенном коде», все по-взрослому =)

[sindrom]

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

[Vidmak]

Интересно, govnokod.ru/, только не агрессивный. Только есть проблемы с авторским правом, и нельзя остальное увидеть. А если можно то это www.codeplex.com

[enej]

Мне больше нравиться идея парного программирования, так как по сути это тоже самое что и обзор кода, вот только лишено некоторых минусов, например:
* не нужно никого просить, у тебя уже есть напарник
* гораздо меньше нужно вносить изменений, потому что ваш напарник сразу же высказывает свои замечания
* дата релиза не давит
Наверное, есть еще, это только те плюсы которые у автора были минусами обзора кода. Хотя не сомневаюсь что этот подход имеет и свои минусы.