Что означает ревью макета

Ревью верстки: 10 полезных замечаний и советов

Примеры ниже написаны с использованием соглашения по именованию селекторов БЭМ, препроцессора Sass и шаблонизатора Jade.

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

Мы пообщались с разработчиками в Noveo и сошлись во мнении, что важно не только тестирование, но и ревью. Особенно важно ревьюить стажеров и новичков, чтобы скорректировать недочеты и не дать сформироваться вредным привычкам. Так, в какой-то момент у наших коллег накопилась небольшая статистика по замечаниям и советам, а как следствие появилась эта статья. Ниже в статье описаны 10 типичных ошибок верстальщиков-новичков и советы по их устранению.

Ошибка 1: Не продумана структура проектной папки

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

Предлагаю с первого же коммита поддерживать прозрачную структуру. Вот пример структуры проектной папки, которую мы использовали на зимней стажировке 2016:

Идёт загрузка… Если ничего так и не загрузилось, то посмотрите пример здесь.

Совсем необязательно в точности копировать эту структуру, достаточно обратить внимание на ряд моментов:

1. Вспомогательные документы, блоки и страницы живут отдельно

В Jade базовый шаблон, блок карусели и страница должны находится в отдельных директориях. Так их будет проще найти. Кроме того, сборщику проекта для компиляции следует «скармливать» только папку со страницами, чтобы не засорять проектную папку ненужными HTML документами для компонент.

В scss общие стили типа reset, или подключения шрифтов, или лейаута верхнего уровня стоит также хранить отдельно от стилей для страниц и блоков. Также целесообразно импортировать все в один документ (например, main.scss) и только этот документ компилировать в css. По объективным причинам таких документов может быть несколько (например, из-за ограничения по количеству селекторов / весу css-документа в IE9 или для поставки специфических стилей для редко встречающихся устройств отдельно от основного документа), но, как и с Jade, не стоит компилировать все подряд.

2. Jade-блоки = БЭМ-блоки

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

В итоге в коде index.jade и других страниц должна быть строчка, указывающая на базовый шаблон, возможно, 1—2 тега верхнего уровня, инклюды и вызов миксинов.

3. Scss зеркалит Jade

Если в Jade-директории blocks есть какой-то документ, то стили для него должны находиться в Scss-директории blocks в документе с таким же названием.

Ошибка 2: копипаста есть, комментариев нет

С копипастой нужно быть особенно аккуратным: вдумчиво все перечитать, исправить «под проект» и удалить лишнее. Однажды мне встретилось:

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

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

Про смешивание пробелов и табов, одинарных и двойных кавычек можно даже не говорить — бездумная копипаста рано или поздно принесет эти прелести в проект (кстати, чтобы поддерживать единый стиль, можно воспользоваться EditorConfig).

У авторов статей есть такой прием — вычитка, когда написанный текст перечитывают несколько раз, чтобы исправить пропущенные ошибки и перефразировать предложения, которые плохо звучат. Рекомендую активно проводить вычитку своего кода и особенно копипасты. Еще в борьбе с копипастой в стилях может очень пригодиться csscomb.

Ошибка 3: блоки влияют друг на друга, одни и те же селекторы встречаются в нескольких местах в scss

Великая сила амперсанда в Sass позволяет писать стили по БЭМ и делать это еще нагляднее. Такой код:

Будет скомпилирован в:

Блок — это отдельный компонент, который не знает о других блоках и их расположении.

«Блок — логически и функционально независимый компонент страницы, аналог компонента в Web Components… Независимость блоков обеспечивает возможность их повторного использования, а также удобство в разработке и поддержке проекта.» (источник)

Такой код будет ошибочным, т.к. состояние одного блока влияет на элемент в другом блоке:

Если возникает необходимость так написать, значит, что-то пошло не так на этапе именования, нужно незамедлительно вернуться к Jade и исправить атрибуты class так, чтобы каждый блок был независимым. Если же заплатка нужна срочно и времени на изменения разметки нет, то оставьте комментарии TODO в обоих документах: в scss и в jade.

Говоря о том, что блок не знает о других блоках, хочется вкратце упомянуть об одном полезном псевдоклассе, о котором постоянно забывают новички. Представьте лейаут верхнего уровня: есть блок с основным контентом и блок aside. Для двух этих блоков указана ширина в %, но на одной странице aside нет и ширина блока с основным контентом должна быть 100%. Тут нам на помощь придет псевдокласс only-child :

Идёт загрузка… Если ничего так и не загрузилось, то посмотрите пример здесь.

В случаях, когда only-child не подходит, используйте модификаторы.

Ошибка 4: не использовать миксины, когда можно использовать миксины

Миксины очень помогают следовать принципу DRY (Don’t Repeat Yourself), их активно используют в scss для таких абстрактных задач, как сетка, но о них почему-то забывают, когда речь заходит о верстке компонент. Например, у вас есть кнопки разных цветов, у которых на ховере происходит инверсия цвета. Для инверсии можно написать миксин и сделать стили чуточку чище:

Идёт загрузка… Если ничего так и не загрузилось, то посмотрите пример здесь.

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

В Jade тоже следует активнее использовать миксины. Например, для svg-спрайтов:

Идёт загрузка… Если ничего так и не загрузилось, то посмотрите пример здесь.

Ошибка 5: беспорядок в медиазапросах

Кроме того, не стоит писать медиазапросы по хардкоду, используйте как минимум переменные для брейкпоинтов. Т.е. вместо:

Переменные для брейкпоинтов стоит хранить в отдельном документе или в layout.scss. Можно пойти дальше, прочитать эту статью и выбрать один из предложенных подходов (или написать свой).

В scss есть возможность вкладывать медиазапросы в правила. Например:

Это будет скомпилировано в:

Так писать медиазапросы довольно удобно, тогда один селектор будет встречаться строго в одном месте в стилях. В таком случае обязательно использовать переменные для основных брейкпоинтов. Должно быть так, чтобы по названиям переменных можно было легко определять, какой брейкпоинт соответствует устройству (по спецификации и макетам проекта), а какой был написан специально под контент.

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

Ошибка 6: не интересоваться UX-проектированием

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

Для начала предлагаю обратить внимание на типичную ошибку новичка: ссылки и кнопки маленького размера, между ними мало пространства. Это ок, если взаимодействие происходит мышкой, но совершенно не годится, когда речь идет о тач-устройствах. Многократные попытки попасть по ссылке, которые в результате приводят к попаданию по соседней ссылке, очень быстро выводят пользователей из себя. Наверняка каждый из нас бывал в такой ситуации, и первое, что хочется сделать, — покинуть неудобный сайт. Чтобы такого не происходило с вашими пользователями, советую почитать гайдлайны от Google или SmashingMagazine.

Ошибка 7: все в пикселях, даже font-size

Советую сделать (если вы еще нее сделали) переход с px на em и rem для таких значений, как:

В рамках БЭМ для блока: rem для размера шрифта блока, em — для остальных величин блока. Для элемента есть 2 стратегии:

Чтобы понять разницу между единицами изменения или выбрать между em и rem, предлагаю обратить внимание на канал DevTips, на котором в весьма непринужденной форме объясняются основы верстки.

Также с псевдоэлементами и em стоит быть осторожней:

«Если в IE9+для одного и того же псевдоэлемента задать размер шрифта через разные селекторы, то он просто перемножит все размеры шрифтов.» (источник)

Ошибка 8: не думать о переполнении и предельных значениях

Верстая по макету, мы имеем дело с идеальной ситуацией. Так новички довольно часто забывают про угрозу переполнения, т.е. когда контент не помещается в блок или элемент и выходит за его пределы. Эту ситуацию нужно срочно исправлять, ведь верстальщик — это еще и своего рода переводчик интерфейса с языка дизайнерского на язык, понятный программистам. Поэтому очень важно в процессе верстки выявить предельные значения объема текста для кнопок, заголовков и прочих элементов с ограниченной вместимостью. Об этих предельных значениях стоит сообщить разработчикам (как минимум написать комментарий в HTML) и по возможности ограничить переполнение в css:

Идёт загрузка… Если ничего так и не загрузилось, то посмотрите пример здесь.

То же касается и изображений. Совсем не факт, что изображения будут загружаться в правильных пропорциях или что они будут обрезаться программно в процессе загрузки. Если гарантии сохранения пропорций нет, то лучше сразу в верстке сделать обертку и применить одну из двух техник: вписывание или растягивание по ширине.

Идёт загрузка… Если ничего так и не загрузилось, то посмотрите пример здесь.

Можно также с надеждой смотреть в сторону object-fit, однако у этого свойства пока довольно слабая поддержка.

Ошибка 9: уделять недостаточно внимания графике

Графику обычно делят на 3 части: иконки, изображения контента, фоновые изображения. С фоновыми изображениями у новичков обычно проблем не возникает, а вот с иконками и контентными изображениями все не так хорошо.

Сегодня стоит предпочесть svg-спрайты иконочным шрифтам, если нет требований к поддержке совсем старых версий IE. Подробное сравнение двух подходов можно прочитать тут и тут. SVG-спрайт тоже можно кешировать, т.е. можно ссылаться на внешний файл, если набор иконок большой и повторяется от страницы к странице. Для кешируемых спрайтов есть полифил, добавляющий поддержку в IE. А в одностраничных приложениях спрайт обычно добавляется в базовый шаблон.

Контентные изображения, например, фото товара, — главная причина большого веса сайтов. На десктопах (особенно на экранах с большой плотностью пикселя) нам нужны изображения в большом разрешении. Их вес обычно не вызывает проблем, т.к. в основном десктопы используют высокоскоростные подключения с неограниченным трафиком. Пользователи мобильных устройств же вынуждены выкачивать лишние мегабайты изображений. А если у них низкая скорость передачи данных, бывают обрывы? Получаем не очень довольных пользователей. Остается только надеяться, что они не в роуминге. Эту ситуацию легко исправить с помощью атрибута scrset у или с помощью тега

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

Ошибка 10: Pixel Perfect? Не, не слышал.

Не зря этот пункт последний в этой статье — о Pixel Perfect часто забывают (кстати, не только новички). С одной стороны, верстальщик почти всегда действует, опираясь на макет, с другой — немножко от этого макета отступает. В процессе работы над проектом какие-то компоненты могут меняться местами, от каких-то отказываются, появляются новые и т.п. Размеры элементов могут немного меняться в зависимости от контента (смотри переполнение и предельные значения). В конце концов, дизайнер может допустить ошибку при выравнивании элементов по сетке. Все это — адекватные причины, чтобы локально отказаться от pixel perfect и верстать «по ситуации». В идеале каждое отступление от макета должно быть задокументировано.

В случаях, когда нет причин отклоняться от макета, стоит четко ему следовать (помочь в этом может, например, расширение для Chrome — Pixel Perfect).

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

Источник

Еще одна статья о code review

Что такое code review

Что можно инспектировать

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

Как проводить review

Вообще, ревью кода должен проводиться в совокупности с другими гибкими инженерными практиками: парное программирование, TDD, CI. В этом случае достигается максимальная эффективность ревью. Если используется гибкая методология разработки, то этап code review можно внести в Definition of Done фичи.

Из чего состоит 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, при этом цель должна быть обозримой и четкой:

Результаты review

Сложности при проведении review (субъективное мнение)

Основная сложность, с которой мы столкнулись, когда внедряли review в первый раз: это невозможность контроля изменений по результатам review. Отчасти это связано с тем, что данная практика применялась без других практик — CI (это еще раз доказывает тот факт, что все инженерные практики должны применяться вместе).

Утилиты для review

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

Ссылки

Пожелания, дополнения, критика приветствуется

Источник

Код ревью: как быть хорошим автором

Привет! Меня зовут Сергей Загурский, я работаю в Joom в команде инфраструктуры. В своей практике ревьюера кода я регулярно сталкиваюсь с тем, что автор не понимает, что ревьюер не является волшебным чёрным ящиком, в который можно закинуть любые изменения и получить по ним обратную связь. Ревьюер, как и автор, будучи человеком, обладает рядом слабостей. И автор должен (если, конечно, он заинтересован в качественном ревью), помочь ревьюеру настолько, насколько это возможно.

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

Что означает ревью макета. Смотреть фото Что означает ревью макета. Смотреть картинку Что означает ревью макета. Картинка про Что означает ревью макета. Фото Что означает ревью макета

Зачем мы делаем ревью кода

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

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

Он показывает концепцию: качественный процесс ревью уменьшает темп роста сложности. И это — главная ценность ревью кода. Это та ценность, которая понятна бизнесу, т. к. речь идёт о скорости разработки, которая напрямую уменьшает стоимость разработки и увеличивает конкурентоспособность. Но если вдруг планируется, что ваш проект не должен надолго пережить точку пересечения графиков, то ревью можно и не вводить.

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

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

Дальше мы сосредоточимся на самой фундаментальной составляющей процесса ревью — на понимании кода.

Как ревьюер делает ревью

Чтобы процесс ревью являлся таковым, ревьюеру требуется, не больше и не меньше, понять суть изменений. Это основа процесса, без которой ревью не имеет никакого смысла. Для понимания требуется время. И это не то количество времени, которое можно потратить в перерыве, «за кофейком». Для понимания требуется время, сопоставимое с тем временем, которое было затрачено на написание кода. Иногда даже больше. Ревью является полноценной работой, наряду с написанием кода. В чём-то даже сложнее.

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

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

Бывает такое, что автор вместе с основными изменениями делает что-то, не связанное с решением своей задачи. Когда ревьюер сталкивается с таким, то он тратит дополнительное время, чтобы понять, как эти изменения связаны с решением основной задачи.

Ещё одним большим пожирателем времени ревьюера является написание замечаний. Да, есть замечания, которые относятся к каким-то, условно, косметическим аспектам. Однако даже их бывает полезно снабдить сопровождающей информацией вроде ссылки на нужную часть документа по стилю кода. И это всё требует времени.

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

Как помочь ревьюеру провести качественное ревью

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

Перед тем, как превратить свои изменения в Pull Request, следует разбить их на логические куски, если в этом есть необходимость. Комфортный объём ревью заканчивается примерно на 500 строках кода изменений. Допустимый — примерно на 1000 строках. Всё, что за пределами 1000 строк, должно быть разбито на более мелкие Pull Request’ы.

Если вы, как и я, не разбиваете свои изменения на куски комфортного размера заранее, то это придётся проделать перед отправкой ваших изменений на ревью. Отмазка, что на это нужно потратить время, не катит. Принимая решение об отправке на ревью 1000+ строк кода, вы, фактически, оцениваете своё время дороже времени ревьюера. По нашим правилам у ревьюера всегда есть право потребовать разбить изменения на более мелкие куски. Мы всегда просим коллег относиться с пониманием, если он этим воспользуется. С опытом становится проще строить свою работу так, чтобы у вас не появлялись гигантские Pull Request’ы, в которых «ничего нельзя отделить».

Отдельно стоит упомянуть изменения, внесённые в код с помощью авторефакторинга или sed’а. Объём таких изменений может быть очень большим. Наличие автоматических изменений вместе с осмысленными изменениями усложняет ревью. Всегда отделяйте авторефакторинги в отдельные Pull Request’ы, если их объём сопоставим с объёмом нового кода, написанного вами.

Следующая часть подготовки состоит в информировании ревьюера о сути изменений. Как я уже писал выше, ревьюеру нужно знать: что сделано, зачем сделано, как сделано, и почему именно так сделано. Часть информации у ревьюера уже может быть. Часть — есть в коде. Всё, что не очевидно из кода, должно быть прописано в описании изменений.

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

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

Итак, автор подготовил изменения к ревью. Пора отправлять ревьюеру? Нет! Тут наступает ещё один важный этап, саморевью. Под саморевью я понимаю именно ревью, осуществлённое самим автором. Не просто беглый визуальный контроль. Нужно постараться провести полноценное ревью. Ответственно проведённое саморевью сэкономит уйму времени ревьюеру. Наверняка в коде есть какие-то остатки отладочных конструкций, закомментированного кода, TODO-комментариев, бессмысленных изменений форматирования, не пригодившегося кода и прочего подобного. Взгляд глазами ревьюера также поможет оценить сложность ревью и даст возможность добавить дополнительной информации к ревью, которая была упущена при подготовке. Пренебрежение автором саморевью я считаю проявлением крайнего неуважения к ревьюеру.

В целом, я считаю допустимым потратить порядка 10% от времени, затраченного на написание кода, на подготовку к ревью. Это время автора, которое мы обменяем на экономию времени ревьюера, и на улучшение качества ревью. Следует помнить, что ревьюеру на качественное ревью легко может потребоваться и 20%, и 50% от времени, затраченного автором на написание кода.

Вот только теперь автор может с чистой совестью отправить изменения ревьюеру.

Дальше начинается жизненный цикл Pull Request’а. Ревьюер должен либо одобрить его, либо попросить внести изменения. Чтобы упростить работу ревьюера, автору стоит добавить комментарий к каждому запрошенному изменению. Это вполне может быть краткое «OK» или «Исправил», если ничего другого не требуется. Убедитесь, что вы понимаете, что просит ревьюер и что вам понятна его аргументация. Не стоит безоговорочно принимать любые запросы на внесение изменений, возводя тем самым ревьюера в околобожественный ранг. Ревью — это обоюдный процесс. Если ревьюеру что-то не понятно, то он спрашивает об этом автора, и наоборот. В случае, если автор не очень опытен, ревьюеру следует приложить особенные усилия к описанию запросов на изменения, чтобы воспользоваться возможностью поделиться с автором своим опытом. Бывают и спорные моменты, когда аргументация недостаточно сильная, чтобы склонить обе стороны к единой точке зрения. С учётом того, что автор находится в более уязвимой позиции, считаю, что при прочих равных преимущество должно оставаться за автором.

Не вносите изменений в свой Pull Request, не относящихся к тому, что попросил вас ревьюер. Это крайне сбивает с толку. Также следует воздержаться от rebase и подобных действий.

Получили одобрение от ревьюера? Отлично, ещё одно качественно написанное и оформленное изменение было добавлено в проект!

Источник

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *