Во-первых, мне тяжело что-либо ревьюить код, если я не вник в суть задачи. Я при ревью спотыкаюсь об вопросы типа, решает ли код подставленную задачу и решает ли он её полностью. Т.е. мне прежде чем ревьюить нужно сначала прочитать задачу, её предысторию (связанные обсуждения) и только потом я могу приступать к коду. Для меня это настоящая работа.
Во-вторых, я спотыкаюсь о то, что я раньше аппрувил задачи с косяками. Такой опыт заставляет немного нервничать, когда курсор зависает над кнопкой Approve.
Как у вас с этим дело обстоит?
Всё сказанное выше — личное мнение, если не указано обратное.
Ф>Во-вторых, я спотыкаюсь о то, что я раньше аппрувил задачи с косяками. Такой опыт заставляет немного нервничать, когда курсор зависает над кнопкой Approve. Ф>Как у вас с этим дело обстоит?
Ревьюайеризм не предполагает ответственности ревьюайериста за то как работает чужой код. Просто накапливается статистика, кто часто ошибается, и кого потом обделить на делёжке добычи по любому другому поводу.
Здравствуйте, Философ, Вы писали:
Ф>Для меня это настоящая работа.
Так это и есть часть настоящей работы.
Ф>Во-вторых, я спотыкаюсь о то, что я раньше аппрувил задачи с косяками.
Это неприятно, но бывает. С другой стороны без ревью косяков бы меньше не стало. А есть опыт, когда на ревью удавалось находить и исправлять проблемы и это радует.
Ф>Как у вас с этим дело обстоит?
Ну да, это серьезная работа. Чем-то она легче чем самому писать, так как тут все написано, надо только прочитать и обдумать. Но чем-то сложнее, т.к. надо достаточно быстро осмыслить и проверить что другой делал куда дольше.
Работа просто другая, но тоже работе и тоже нужная — это возможность убрать баг, и не разбираться с ним потом в спешке готовя фикс.
O>>Ревьюайеризм не предполагает ответственности ревьюайериста... Ф>Можно не читать — просто жмякать Approve?
Либо делаешь бесплатно эту дополнительную работу непонятной стоимости (непонятно сколько туда надо внимания вложить чтобы не пропустить неизвестные проблемы — значит для работодателя есть вероятность, что работник вложит по собственной инициативе/из страха гораздо больше, чем если с ним явно договариваться), либо жмёшь Approve на свой постепенный переход в статус неполноправного и отчуждённого от карьеры.
Так в чем все-таки проблема, не хочешь ревьюить в принципе или не хочешь тратить на это непрогнозируемое количество времени?
Если первое — становись менеджером, будешь делегировать ревью другим.
Если второе — трекай в задаче время, потраченное на ревью. И установи какой-то лимит, который допустимо потратить на ревью одной задачи. Не уложился — ну что ж, чем смог — помог.
Здравствуйте, Философ, Вы писали:
Ф>Как у вас с этим дело обстоит?
В нашей комманде, любой знает, что если ты делаешь какой то адское изменение, которая затрагивает кучу кода, логика не тривиальная. То уже на начальном этапе разработке, думает парарельно кто это, и как будет ревьюить. Ну, соответственно время ревьювера также дополнительно оценивается и включается в планирование, чтобы его загруженность дала ему возможность это все адекватно отревьюить.
Если кто то под конец спринта, придет с каким то адским ревью, который нужно дня два потратить. То будут конечно разборки, почему заранее не предупредил, почему так сложно. А дальше вопрос приоритетов, кого то срывать с его задач, если эта фича важнее. Или фича ждет пока кто то освободиться.
Отдельно всегда на ревью, лично я смотрю можно ли все это было сделать проще.
Здравствуйте, Философ, Вы писали:
Ф>Во-первых, мне тяжело что-либо ревьюить код, если я не вник в суть задачи. Я при ревью спотыкаюсь об вопросы типа, решает ли код подставленную задачу и решает ли он её полностью.
Потребуй сопроводить код тестами, которые четко описывают все кейсы. Потребуй наличие юзерстори или юзкейса.
Если это багфикс, потребуй описания шагов бага, и также наличия теста на данный сценарий.
Если тесты будут написаны непонятно, потребуй, чтобы их написал понятно и наглядно (например, в теста на rest api наглядность теста повышает замена точечных ассертов на полный json реквеста и респонза).
Здравствуйте, Философ, Вы писали:
Ф>Во-первых, мне тяжело что-либо ревьюить код, если я не вник в суть задачи.
А зачем это нужно? Если сильно хочется вникать да контролировать плотно, то выгоднее идти через парно-программирование.
Если такой роскоши нет, то на ревью можно завести чек-лист, и каждый ревьюер будет идти по нем.
— размер пр. Если слишком много — пусть автор заводит фича бранч, сплитает фичу на промежуточные пр.
— есть ли валидация параметров публичных методов
— есть ли тесты — юниты, функциональные, e2e
— корректна ли документация на методы
— изобретены ли велосипеды
— именование публичных методов
— есть ли обработка ошибок
— корректно ли оформлены конфиги
— есть ли магические константы итд
— мутный код вида "а херлиты в базу лезешь сразу из контролера"
— секурити типа "распаковывать jwt токен без валидации ой-ой-ой" создан тикет №10123
До кучи, правила ревьюеров, запретить всё ниже
— переделай, измени, убери — вместо этого интервьюер сообщает, какая проблема с кодом, если надо, то создаёт тикет. Если же хочется идти по варианту "переделай, измени, убери", то парно-программирование будет лучше.
— это не секюрно. Вместо этого интервьюер сообщает, почему не секюрно, и создает тикет.
— идиотский код. Вместо этого интервьюер задает вопросы, уточняет и тд.
— нет форматирования итд. Для этого есть автоматические тулы
То есть, вместо два-три дня на ревью быстрый проход по чеклисту, на выходе список тикетов. Мелкие и критические тикеты исправляются сразу, остальные уходят в баклог как технические задачи. Дальше ПР мергается в транк.
Идея в том, что бы кодревью прошел быстро, и бранча не тухла неделями. Иначе проблем она создаст гораздо больше. Чем позже мерж, тем больше геморроя при мерже-ребейзе и тем больше скрытых багов
Здравствуйте, Философ, Вы писали:
Ф>Как у вас с этим дело обстоит?
Аналогично. Но даже низкое качество ревью и просто попытка разобраться помогают автору переделать всё так, чтоб более грамотный (и более занятый) сотрудник потратит меньше своего ценного времени на это и больше времени на твой собственный код. И, глядя на чужие косяки, как бы сам слегка растёшь над собой.
Здравствуйте, Философ, Вы писали:
Ф>Как у вас с этим дело обстоит?
плохо
мне часто предлагают ревьювить код который реализует бизнес логику или GUI на java
а мой код на Си обрабатывающий видео поток отдают на ревью java программистам
Ф>Во-первых, мне тяжело что-либо ревьюить код, если я не вник в суть задачи. Я при ревью спотыкаюсь об вопросы типа, решает ли код подставленную задачу и решает ли он её полностью. Т.е. мне прежде чем ревьюить нужно сначала прочитать задачу, её предысторию (связанные обсуждения) и только потом я могу приступать к коду. Для меня это настоящая работа.
Ф>Во-вторых, я спотыкаюсь о то, что я раньше аппрувил задачи с косяками. Такой опыт заставляет немного нервничать, когда курсор зависает над кнопкой Approve.
Ф>Как у вас с этим дело обстоит?
Спокойно к этому отношусь.
Надо понимать, что качественное ревью, со вниканием во все детали — это как заново написать этот код. Слишком трудоемко и обычно не требуется. Смотрю обычно явную лажу или какие-то типовые косяки у новичков. Ну что бы что-нибудь не отломали в моём коде. Глубоко не вникаю.
Здравствуйте, sergey2b, Вы писали:
S>мне часто предлагают ревьювить код который реализует бизнес логику или GUI на java S>а мой код на Си обрабатывающий видео поток отдают на ревью java программистам
Капец какой бардак!
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Забанили по IP, значит пора закрыть эту страницу.
Всем пока
Здравствуйте, Tourist, Вы писали:
T>Если кто то под конец спринта, придет с каким то адским ревью, который нужно дня два потратить. То будут конечно разборки, почему заранее не предупредил, почему так сложно. А дальше вопрос приоритетов, кого то срывать с его задач, если эта фича важнее. Или фича ждет пока кто то освободиться.
Забавно бывает. То на задаче где просто ресурс надо было поправить кто-то докапывается до какой-то мелочи, а потом вливаешь какой-то неслабый кусок функционала — и все такие ну давай мёржить, а потом пусть тестеры баги заводят ну это было в старом проекте, где на платформе Eclipse пилили RCP/RAP приложение. Сейчас на проектах уже обычно не так, а тут в эффектор вливаюсь и чёт пока прям сложненько, больно сильно логика размазана...
Здравствуйте, CreatorCray, Вы писали:
S>>а мой код на Си обрабатывающий видео поток отдают на ревью java программистам CC>Капец какой бардак!
ну а если больше нет си-шных программистов, надо же что-то делать
Когда мы старый проект индусам передавали, рассказывали им про всякие разные части проекта — и вот в очередной раз когда рассказываем про очередной кусок, который представляет собой тотальный редизайн, на половине! рассказа где-то через минут 45 прибегает в конфу менеджер индусский и говорит — ой это конечно всё хорошо что вы тут рассказываете, но эти индусы в этом ничего не понимают. Давайте позже эту встречу повторим, мы найдём тех, кто в этом рубит
всё конечно кончилось просто тем, что тупо час скринкаста записали, в конфлюенс залили и всё
Здравствуйте, CreatorCray, Вы писали:
S>>мне часто предлагают ревьювить код который реализует бизнес логику или GUI на java S>>а мой код на Си обрабатывающий видео поток отдают на ревью java программистам CC>Капец какой бардак!
Это состояние индустрии такое. Например, в команде джавист, сиплюсник и пейтон. Это препятствие для код ревью, но вовсе не повод отказываться от него.
Здравствуйте, Философ, Вы писали:
Ф>Во-первых, мне тяжело что-либо ревьюить код, если я не вник в суть задачи. Я при ревью спотыкаюсь об вопросы типа, решает ли код подставленную задачу и решает ли он её полностью.
Проверка работоспособности изменений на QA. Ревью кода- это незамыленный взгляд со стороны.
Я вот сталкиваюсь, что подчинённые неохотно ревьювят мои PR, а когда задают правильные вопросы- я не всегда переделываю. За что и поплатился недавно регрессией в продакшене.
Здравствуйте, Философ, Вы писали:
Ф>Как у вас с этим дело обстоит?
Я считаю, что вопросы о "решает и решает ли полностью" это зона ответственности тестировщиков, а не ревьюеров. Ревью это проверка соответствия формальным параметрам, которые не удалось автоматизировать. Ну и шанс найти баги, которые можно увидеть "свежим глазом". Если я буду в каждую задачу погружаться, по которой делаю ревью, то я больше ничем заниматься не буду.
По идее погружение это про парное программирование больше.
Хотя, конечно, тут вопрос больше к менеджерам. Если на ревью выделяется достаточно времени (процентов 20-50 от времени выполнения задачи) — ну классно.
