Вообще интересная тема, возможно избитая, но полезная.
Если кому интересно как проводятся код ревью в гугле, тому сюда:
Если не охота слушать заморской мовы и хочется вкратце и в картинках, то сюда:
http://habrahabr.ru/post/215119/
http://habrahabr.ru/post/142564/
Так же, еще очень интересен следующий линк:
https://toster.ru/q/15353
В котором девелоперы делятся своим опытом и отвечают на вопросы ниже:
http://habrahabr.ru/post/142564/
Так же, еще очень интересен следующий линк:
https://toster.ru/q/15353
В котором девелоперы делятся своим опытом и отвечают на вопросы ниже:
Кто практикует code-review? Поделитесь опытом
Поделитесь опытом, как устроен процесс разработки?
- Какой код ревьювится? (весь/отдельные компоненты или изменения/другой вариант)
- Какие используете инструменты?
- Как код попадает на review? (всегда при коммите, создаёте review request руками когда нужно проревьювить, другой вариант?)
- Как используются результаты review?
- Как отслеживается что замечания сделанные во время review были исправлены?
=======================================
Code review - инженерная практика в терминах гибкой методологии разработки. Это анализ (инспекция) кода с целью выявить ошибки, недочеты, расхождения в стиле написания кода, в соответствии написанного кода и поставленной задачи.
К очевидным плюсам этой практики можно отнести:
Самая значимая вещь, которая делает код в компании Google таким хорошим проста — code review (далее CR). Google не единственная компания, использующая CR. Всем известно, что это хорошая идея и множество разработчиков делают это. Но я не видел ни одной другой большой компании, в которой CR был бы так грамотно внедрен. В Google ни одна линия кода не уходит в production пока не получит позитивную оценку на CR.
Вы должны уделять внимание CR. Это непременное условие для разработки любого программного обеспечения на серьезном уровне. CR должен применяться не только для кода который уходит в production, но вообще для всего. Это не такая большая разница во времени как вы думаете, но разница в качестве получается огромна.
Что же вы получите от использования CR?
Очевидное: одна голова хорошо, а две лучше. Как минимум еще один человек просмотрит ваш код перед тем, как он будет проверен в реальных условиях (баги). Это наиболее часто упоминающийся профит от CR. Но мой опыт говорит, что это как раз менее всего ценно. Коллеги действительно находят ошибки в вашем коде, но подавляющее большинство таких ошибок найденных в процессе code review, прямо говоря, тривиальные вещи, которые автор кода найдет за несколько минут. Баги, на которые действительно нужно потратить время не ловятся на CR.
Самое большая польза от code review носит социальный характер. Если вы программист и вы знаете, что ваши коллеги обязательно посмотрят ваш код, вы размышляете по-другому. Вы будете писать аккуратный, хорошо документированный и организованный код, потому как вы знаете, что люди, мнение о коде которых вам важно, внимательно проверят отправленный на review код. Без CR, вы также знаете, что другие люди посмотрят ваш код. Но вы понимаете, что это произойдет не сразу, поэтому этот факт не оказывает такого подсознательного эффекта.
Другое большое преимущество — это распространение знаний. Во многих командах, у каждого отдельного программиста есть кусок кода в проекте, за который он ответственен, и этот программист сфокусирован именно на этот код. Пока коллеги не поломают что-то связанное с этим кодом, они на него не обращают внимания. Побочный эффект от этого в том, что для каждого компонента существует только один человек, который полностью понимает как он работает. Если этот человек не укладывается в сроки или — упаси Боже — покидает компанию, не останется никого, кто знаком с кодом компонента. С CR будет как минимум два человека, которые знакомы с кодом — автор и reviewer. Reviewer не знает код так, как его знает автор, но он знаком со структурой и архитектурой кода, что очень значимо.
Конечно же, это не просто. Из моего опыта могу сказать, что нужно время чтобы наладить хороший процесс CR. Я знаю несколько подводных камней, которые могут вызвать проблемы. Пока эти камни периодически вылезают CR может оставаться на плохом уровне, быть в тягость и препятствовать принятию в ежедневный workflow.
Главное правило — целью CR является нахождение проблем в коде перед тем как он будет закоммичен, т.е. правильность. Самая распространенная ошибка в CR у новичков это проверять код, основываясь на том как бы это сделал он. Обычно существует несколько путей решения одного и того же. Помимо этого, есть миллион способов написать задуманное в виде кода. Задача reviewerа, не сделать так, чтобы написанный код был похож на то, чтобы он написал, нет — он никогда не будет таким. Задача reviewerа — удостовериться в том, что код, написанный автором корректен. Когда нарушается это правило, вы в итоге придете к эмоциям и переживаниям, что не является тем, к чему вы стремитесь.
Дело в том, что это совершенно естественная ошибка. Если вы программист, когда вы смотрите на проблему в коде, вы можете сразу видеть способ ее решить и вы думаете, что то, что вы увидели и есть решение. Но это не так, и чтобы стать хорошим reviewerом нужно это понять.
Второй по значимости подводный камень то, что люди чувствуют необходимость сказать что-либо. Вы знаете, что автор потратил достаточно времени и сил, работая над кодом и вроде как вы должны высказать свое мнение.
Нет, вы не должны.
Вполне нормально просто сказать «Выглядит хорошо». Если вы постоянно пытаетесь найти что-нибудь покритиковать, тогда все чего вы добьетесь так это подорвать свой авторитет и поломать отношения в команде. Когда вы критикуете код только для того чтобы найти что сказать, тогда люди чей код вы проверяете поймут, что вы пишете только для того чтобы что-то написать и ваши комментарии не будут воспринимать всерьез.
Третье — скорость. Не нужно сломя голову бежать проверять только что посланный на review код, но с другой стороны нужно делать это быстро. Ваши коллеги ждут вас. Если вы и ваши коллеги не уделяете время тому, чтобы CR был сделан, причем сделан быстро, тогда CR может стать причиной недовольства людей в коллективе. Может показаться, что это переключение фокуса — взяться за CR. Это не совсем так. Не нужно бросать все в тот момент, когда новый код послан на review. Но в течение нескольких часов вы совершенно точно сделаете паузу для того, чтобы попить что-нибудь, сходить в туалет или просто походить. Когда вы вернетесь с паузы уделите внимание CR. Если вы возьмете это в привычку, то никто в команде не будет подолгу ожидать ваш review и он не будет доставлять никакого дискомфорта в работе.
Для ревью подходит любой код. Однако, review обязательно должно проводиться для критических мест в приложении (например: механизмы аутентификации, авторизации, передачи и обработки важной информации — обработка денежных транзакций и пр.).
Также для review подходят и юнит тесты, так как юнит тесты — это тот же самый код, который подвержен ошибкам, его нужно инспектировать также тщательно как и весь остальной код, потому что, неправильный тест может стоить очень дорого.
Вообще, ревью кода должен проводиться в совокупности с другими гибкими инженерными практиками: парное программирование, TDD, CI. В этом случае достигается максимальная эффективность ревью. Если используется гибкая методология разработки, то этап code review можно внести в Definition of Done фичи.
Также очень важно определиться, за кем будет последнее слово в принятии финального решения в случае возникновения спора. Обычно, приоритет отдается тому кто будет реализовывать код (как в scrum при проведении planning poker), либо специальному человеку, который отвечает за этот код (как в google — code owner).
Design review можно проводить за столом, в кругу коллег, у маркерной доски, в корпоративной wiki. На design review тот, кто будет писать код, расскажет о выбранной стратегии (примерный алгоритм, требуемые инструменты, библиотеки) решения поставленной задачи. Вся прелесть этого этапа заключается в том, что ошибка проектирования будет стоить 1-2 часа времени (и будет устранена сразу на review).
Можно проводить code review разными способами — дистанционно, когда каждый разработчик сидит за своим рабочим местом, и совместно — сидя перед монитором одного из коллег, либо в специально выделенным для этого месте, например meeting room. В принципе существует много способов (можно даже распечатать исходный код и вносить изменения на бумаге).
Данный вид review проводится перед внесением изменений в VCS. Этот подход позволяет содержать в репозитории только проверенный код. В microsoft используется этот подход: всем участникам review рассылаются патчи с изменениями. После того как собран и обработан фидбэк, процесс повторяется до тех пор пока все ревьюверы не согласятся с изменениями.
Данный вид review проводится после внесения изменений в VCS. При этом можно коммитить как в основную ветвь, так и во временную ветку (а в основную ветку вливать уже проверенные изменения).
Можно также проводить тематические code review — их можно использовать как переходный этап на пути к полноценному code review. Их можно проводить для критического участка кода, либо при поиске ошибок. Самое главное — это определить цель данного review, при этом цель должна быть обозримой и четкой:
Основное отличие тематических review от полноценного code review — это их узкая специализация. Если в code review мы смотрим на стиль кода, соответствие реализации и постановки задачи, поиск опасного кода, то в тематическом review мы смотрим обычно только один аспект (чаще всего — анализ алгоритма на соответствие ТЗ, обработка ошибок).
Преимущество такого подхода заключается в том, что команда постепенно привыкает к практике review (его можно использовать нерегулярно, по требованию). Получается некий аналог мозгового штурма. Мы использовали такой подход при поиске логических ошибок в нашем ПО: смотрели «старый» код, который был написан за несколько месяцев до review (это можно отнести тоже к отличиям от обычного review — где обычно смотрят свежий код).
Самое главное при проведении review — это использование полученного результата. В результате review могут появиться следующие артефакты:
При этом очень важно, чтобы все результаты не пропали, и были внесены в VCS, wiki. Этому могут препятствовать:
Для преодоления этих проблем частично может помочь:
Основная сложность, с которой мы столкнулись, когда внедряли review в первый раз: это невозможность контроля изменений по результатам review. Отчасти это связано с тем, что данная практика применялась без других практик — CI (это еще раз доказывает тот факт, что все инженерные практики должны применяться вместе).
Вообще, утилит для проведения review существует большое количество, как платных, так и бесплатных. Я не стал их приводить, чтобы не навязывать свою точку зрения, в интернете можно найти множество инструментов и подробное описание.
Что такое code review
Code review - инженерная практика в терминах гибкой методологии разработки. Это анализ (инспекция) кода с целью выявить ошибки, недочеты, расхождения в стиле написания кода, в соответствии написанного кода и поставленной задачи.
К очевидным плюсам этой практики можно отнести:
- Улучшается качество кода
- Находятся «глупые» ошибки (опечатки) в реализации
- Повышается степень совместного владения кодом
- Код приводится к единому стилю написания
- Хорошо подходит для обучения «новичков», быстро набирается навык, происходит выравнивание опыта, обмен знаниями
Самая значимая вещь, которая делает код в компании Google таким хорошим проста — code review (далее CR). Google не единственная компания, использующая CR. Всем известно, что это хорошая идея и множество разработчиков делают это. Но я не видел ни одной другой большой компании, в которой CR был бы так грамотно внедрен. В Google ни одна линия кода не уходит в production пока не получит позитивную оценку на CR.
Вы должны уделять внимание CR. Это непременное условие для разработки любого программного обеспечения на серьезном уровне. CR должен применяться не только для кода который уходит в production, но вообще для всего. Это не такая большая разница во времени как вы думаете, но разница в качестве получается огромна.
Что же вы получите от использования CR?
Очевидное: одна голова хорошо, а две лучше. Как минимум еще один человек просмотрит ваш код перед тем, как он будет проверен в реальных условиях (баги). Это наиболее часто упоминающийся профит от CR. Но мой опыт говорит, что это как раз менее всего ценно. Коллеги действительно находят ошибки в вашем коде, но подавляющее большинство таких ошибок найденных в процессе code review, прямо говоря, тривиальные вещи, которые автор кода найдет за несколько минут. Баги, на которые действительно нужно потратить время не ловятся на CR.
Самое большая польза от code review носит социальный характер. Если вы программист и вы знаете, что ваши коллеги обязательно посмотрят ваш код, вы размышляете по-другому. Вы будете писать аккуратный, хорошо документированный и организованный код, потому как вы знаете, что люди, мнение о коде которых вам важно, внимательно проверят отправленный на review код. Без CR, вы также знаете, что другие люди посмотрят ваш код. Но вы понимаете, что это произойдет не сразу, поэтому этот факт не оказывает такого подсознательного эффекта.
Другое большое преимущество — это распространение знаний. Во многих командах, у каждого отдельного программиста есть кусок кода в проекте, за который он ответственен, и этот программист сфокусирован именно на этот код. Пока коллеги не поломают что-то связанное с этим кодом, они на него не обращают внимания. Побочный эффект от этого в том, что для каждого компонента существует только один человек, который полностью понимает как он работает. Если этот человек не укладывается в сроки или — упаси Боже — покидает компанию, не останется никого, кто знаком с кодом компонента. С CR будет как минимум два человека, которые знакомы с кодом — автор и reviewer. Reviewer не знает код так, как его знает автор, но он знаком со структурой и архитектурой кода, что очень значимо.
Конечно же, это не просто. Из моего опыта могу сказать, что нужно время чтобы наладить хороший процесс CR. Я знаю несколько подводных камней, которые могут вызвать проблемы. Пока эти камни периодически вылезают CR может оставаться на плохом уровне, быть в тягость и препятствовать принятию в ежедневный workflow.
Главное правило — целью CR является нахождение проблем в коде перед тем как он будет закоммичен, т.е. правильность. Самая распространенная ошибка в CR у новичков это проверять код, основываясь на том как бы это сделал он. Обычно существует несколько путей решения одного и того же. Помимо этого, есть миллион способов написать задуманное в виде кода. Задача reviewerа, не сделать так, чтобы написанный код был похож на то, чтобы он написал, нет — он никогда не будет таким. Задача reviewerа — удостовериться в том, что код, написанный автором корректен. Когда нарушается это правило, вы в итоге придете к эмоциям и переживаниям, что не является тем, к чему вы стремитесь.
Дело в том, что это совершенно естественная ошибка. Если вы программист, когда вы смотрите на проблему в коде, вы можете сразу видеть способ ее решить и вы думаете, что то, что вы увидели и есть решение. Но это не так, и чтобы стать хорошим reviewerом нужно это понять.
Второй по значимости подводный камень то, что люди чувствуют необходимость сказать что-либо. Вы знаете, что автор потратил достаточно времени и сил, работая над кодом и вроде как вы должны высказать свое мнение.
Нет, вы не должны.
Вполне нормально просто сказать «Выглядит хорошо». Если вы постоянно пытаетесь найти что-нибудь покритиковать, тогда все чего вы добьетесь так это подорвать свой авторитет и поломать отношения в команде. Когда вы критикуете код только для того чтобы найти что сказать, тогда люди чей код вы проверяете поймут, что вы пишете только для того чтобы что-то написать и ваши комментарии не будут воспринимать всерьез.
Третье — скорость. Не нужно сломя голову бежать проверять только что посланный на review код, но с другой стороны нужно делать это быстро. Ваши коллеги ждут вас. Если вы и ваши коллеги не уделяете время тому, чтобы CR был сделан, причем сделан быстро, тогда CR может стать причиной недовольства людей в коллективе. Может показаться, что это переключение фокуса — взяться за CR. Это не совсем так. Не нужно бросать все в тот момент, когда новый код послан на review. Но в течение нескольких часов вы совершенно точно сделаете паузу для того, чтобы попить что-нибудь, сходить в туалет или просто походить. Когда вы вернетесь с паузы уделите внимание CR. Если вы возьмете это в привычку, то никто в команде не будет подолгу ожидать ваш review и он не будет доставлять никакого дискомфорта в работе.
Что можно инспектировать
Для ревью подходит любой код. Однако, review обязательно должно проводиться для критических мест в приложении (например: механизмы аутентификации, авторизации, передачи и обработки важной информации — обработка денежных транзакций и пр.).
Также для review подходят и юнит тесты, так как юнит тесты — это тот же самый код, который подвержен ошибкам, его нужно инспектировать также тщательно как и весь остальной код, потому что, неправильный тест может стоить очень дорого.
Как проводить review
Вообще, ревью кода должен проводиться в совокупности с другими гибкими инженерными практиками: парное программирование, TDD, CI. В этом случае достигается максимальная эффективность ревью. Если используется гибкая методология разработки, то этап code review можно внести в Definition of Done фичи.
Из чего состоит review
- Сначала design review — анализ будущего дизайна (архитектуры).Данный этап очень важен, так как без него ревью кода будет менее полезным или вообще бесполезным (если программист написал код, но этот код полностью неверен — не решает поставленную задачу, не удовлетворяет требованиям по памяти, времени). Пример: программисту поставили задачу написать алгоритм сортировки массива. Программист реализовал алгоритм bogo-sort, причем с точки зрения качества кода — не придраться (стиль написания, проверка на ошибки), но этот алгоритм совершенно не подходит по времени работы. Поэтому ревью в данном случае бесполезно (конечно — это утрированный пример, но я думаю, суть ясна), здесь необходимо полностью переписывать алгоритм.
- Собственно, сам code review — анализ написанного кода. На данном этапе автору кода отправляются замечания, пожелания по написанному коду.
Также очень важно определиться, за кем будет последнее слово в принятии финального решения в случае возникновения спора. Обычно, приоритет отдается тому кто будет реализовывать код (как в scrum при проведении planning poker), либо специальному человеку, который отвечает за этот код (как в google — code owner).
Как проводить design review
Design review можно проводить за столом, в кругу коллег, у маркерной доски, в корпоративной wiki. На design review тот, кто будет писать код, расскажет о выбранной стратегии (примерный алгоритм, требуемые инструменты, библиотеки) решения поставленной задачи. Вся прелесть этого этапа заключается в том, что ошибка проектирования будет стоить 1-2 часа времени (и будет устранена сразу на review).
Как проводить code review
Можно проводить code review разными способами — дистанционно, когда каждый разработчик сидит за своим рабочим местом, и совместно — сидя перед монитором одного из коллег, либо в специально выделенным для этого месте, например meeting room. В принципе существует много способов (можно даже распечатать исходный код и вносить изменения на бумаге).
Pre-commit review
Данный вид review проводится перед внесением изменений в VCS. Этот подход позволяет содержать в репозитории только проверенный код. В microsoft используется этот подход: всем участникам review рассылаются патчи с изменениями. После того как собран и обработан фидбэк, процесс повторяется до тех пор пока все ревьюверы не согласятся с изменениями.
Post-commit review
Данный вид review проводится после внесения изменений в VCS. При этом можно коммитить как в основную ветвь, так и во временную ветку (а в основную ветку вливать уже проверенные изменения).
Тематические review
Можно также проводить тематические code review — их можно использовать как переходный этап на пути к полноценному code review. Их можно проводить для критического участка кода, либо при поиске ошибок. Самое главное — это определить цель данного review, при этом цель должна быть обозримой и четкой:
- "Давайте поищем ошибки в этом модуле" — не подходит в качестве цели, так как она необозрима.
- "Анализ алгоритма на соответствие спецификации RFC 1149" — уже лучше.
Основное отличие тематических review от полноценного code review — это их узкая специализация. Если в code review мы смотрим на стиль кода, соответствие реализации и постановки задачи, поиск опасного кода, то в тематическом review мы смотрим обычно только один аспект (чаще всего — анализ алгоритма на соответствие ТЗ, обработка ошибок).
Преимущество такого подхода заключается в том, что команда постепенно привыкает к практике review (его можно использовать нерегулярно, по требованию). Получается некий аналог мозгового штурма. Мы использовали такой подход при поиске логических ошибок в нашем ПО: смотрели «старый» код, который был написан за несколько месяцев до review (это можно отнести тоже к отличиям от обычного review — где обычно смотрят свежий код).
Результаты review
Самое главное при проведении review — это использование полученного результата. В результате review могут появиться следующие артефакты:
- Описание способа решения задачи (design review)
- UML диаграммы (design review)
- Комментарии к стилю кода (code review)
- Более правильный вариант (быстрый, легкочитаемый) реализации (design review, code review)
- Указание на ошибки в коде (забытое условие в switch, и т.д.) (code review)
- Юнит тесты (design review, code review)
При этом очень важно, чтобы все результаты не пропали, и были внесены в VCS, wiki. Этому могут препятствовать:
- Сроки проекта.
- Лень, забывчивость разработчиков
- Отсутствие удобного механизма внесения изменений review, а также контроль внесения этих изменений.
Для преодоления этих проблем частично может помочь:
- pre-commit hook в VCS
- Создание ветви в VCS, из которой изменения вливаются в основную ветвь только после review
- Запрет сборки дистрибутива на CI сервере без проведения review. Например, при сборке дистрибутива проверять специальные свойства (svn:properties), либо специальный файл с результатами review. И отказывать в сборке дистрибутива, если не все ревьюверы одобрили (approve) код.
- Использование методологии в разработке (в которой code review является неотъемлемой частью).
Сложности при проведении review (субъективное мнение)
Основная сложность, с которой мы столкнулись, когда внедряли review в первый раз: это невозможность контроля изменений по результатам review. Отчасти это связано с тем, что данная практика применялась без других практик — CI (это еще раз доказывает тот факт, что все инженерные практики должны применяться вместе).
Утилиты для review
Вообще, утилит для проведения review существует большое количество, как платных, так и бесплатных. Я не стал их приводить, чтобы не навязывать свою точку зрения, в интернете можно найти множество инструментов и подробное описание.
No comments:
Post a Comment