Комментарии 134
исправить потом, если это реально большая работа, лучше просто отдельной таской завести.
Хм… Если исправить вносимые изменения — это большая работа, значит MR был слишком большой. А если MR слишком большой, то наверно и таск был немаленький. Имхо надо декомпозировать такие задачи
Не, не факт. Ты можешь сделать небольшой ПР, который использует большой модуль, который лучше не использовать, и тебя просят сделать новый. Вот в таких случаях действительно лучше всего создавать отдельную таску.
Часто бывает так: есть решение проблемы для 99% случаев и есть идея как решить ещё 1% случаев, но это потребует ещё неделю на исследование и не факт что вообще удастся. И вот надо решить: релизить сейчас с на 99% работающим функционалом или задерживать на неделю ради призрачного шанса 100%-идеального кода.
Наверное, тут имеется ввиду исправлять потом код из нового PR, а не старый код. Конечно, не стоит просить автора PR, который поправил только одну строчку в функции, править ее всю, это лучше делать отдельным PR'ом.
Слабо представляю как это будет работать. Завести таской с каким приоритетом вы предлагаете?
Если кто-то через несколько месяцев реально начнет выполнять эту таску и код поломается, есть риск получить по шапке от бизнес пользователей с формулировкой — «зачем было трогать код который давно и успешно работает»
Если кто-то через несколько месяцев реально начнет выполнять эту таску и код поломается, есть риск получить по шапке от бизнес пользователей с формулировкой — «зачем было трогать код который давно и успешно работает»
Такие таски в рамках рефакторинга потом постепенно закрываются, в конце концов бизнес должен понимать, что многие фичи делаются в долг и что тех долг нужно выплачивать, даже если сейчас все работает.
Как по мне, стратегия tick/tack интела в данном случае самая выигрышная.
Ну бизнес смысла в такой таске нет никакого. Т.е. у вас ситуация — был написан код, он прошел кодеревью(один из этапов контроля качества проекта), зачем создавать таску на его переделку? Это видится просто попыткой занять чем-то людей, нормальный прожект менеджер или лид должен такое пресекать
Вообще это часть декомпозиции задачи, когда ты не забываешь, что какие то части фич заткнул костылями, потому что нет времени на общее решение
Если у вас нет времени сделать это правильно, то сделать это дважды — времени точно не будет. Потому что с точки зрения накопления техдолга, самая выгодная позиция у вас — именно сегодня, пока долг наименьший.
Тут всё зависит от того, что за код вы пишите. Разумеется делать так, чтобы в новом кода, который вы обсуждаете, можно было что-то улучшить — и это не сделано… не стоит.
Однако гораздо чаще ситуация бывает другой: можно было бы сделать код лучше… если бы уже существующий код не опирался на какой-то странное и неожиданное свойство (ну например мы хотим что-то распараллелить — а там функция с глобалами, которая это сделать не даёт).
Вот тут — уже нужно взвешивать все варианты.
Иногда — лучше исправить существующий код.
Иногда — стоит завести таску.
Но никогда не стоит ограничиваться просто ToDo: если возможное исправление слишком мало, чтобы оформлять его отдельной таской и обсуждать с другими… то оно явно слишком мало, чтобы его не исправлять. Исправьте и забудьте — тут я согласен.
Однако гораздо чаще ситуация бывает другой: можно было бы сделать код лучше… если бы уже существующий код не опирался на какой-то странное и неожиданное свойство (ну например мы хотим что-то распараллелить — а там функция с глобалами, которая это сделать не даёт).
Вот тут — уже нужно взвешивать все варианты.
Иногда — лучше исправить существующий код.
Иногда — стоит завести таску.
Но никогда не стоит ограничиваться просто ToDo: если возможное исправление слишком мало, чтобы оформлять его отдельной таской и обсуждать с другими… то оно явно слишком мало, чтобы его не исправлять. Исправьте и забудьте — тут я согласен.
Разумеется делать так, чтобы в новом кода, который вы обсуждаете, можно было что-то улучшить
Тут, правда, следует отличать термины «улучшить» от «эволюционировать». Рефакторится не только плохой код, но и хороший код, который практикует принципы YAGNI и Evolutionary Design — необычайно мощные методики, дающие колоссальный прирост в темпах разработки.
Но нужно отличать рефакторинг с целью развития программы (т.е. эволюция решений, принятых ранее в условиях недостаточной информированности) от рефакторинга с целью повышения внутреннего качества и экономики разработки. Код должен поставляться всегда качественным, в то же время, с точки зрения YAGNI и Evolutionary Design, он не обязательно должен быть завершенным.
Существует простая лакмусовая бумажка принципа YAGNI: «выделение лишних абстракций (и любое другое усложнение) оправдано лишь в том случае, если стоимость их выделения в будущем будет существенно дороже, чем сейчас».
стоимость их выделения в будущем будет существенно дороже, чем сейчас
Только не очень просто оценить стоимость в будущем. Половина костылей и легаси рождаются именно под девизом: «мы же это не будем менять „никогда“, так зачем тратить время на лишний уровень абстракций или вообще на разбиение на классы и слои».
Если вы работаете в условиях полной информированности — то нет проблем. Но гораздо чаще приходится работать в условиях недостаточной информированности. Если бы это было не так, то все до сих пор писали бы по BDUF, а не по Agile. И здесь, как раз, чаще случается обратное явление — предполагаемая потребность была реализована, потребляла ресурсы на сопровождение, но так и осталась невостребованной. Или же она оказалась востребованной, но через значительное время, т.е. потраченные ресурсы все это время были «заморожены» и не приносили выгоды.
Именно эту проблему и решают YAGNI и Evolutionary Design — достижение наилучшей экономики разработки в условиях недостаточной информированности.
А поэтому, если решение ухудшает экономику разработки, то это не YAGNI, так как оно противоречит его назначению. Например, нарушение Open–closed principle не есть YAGNI. Хороший архитектор, наоборот, максимизирует количество неприятных решений.
Воздержусь от обсуждений, выходящих за пределы темы статьи. Оценивание — это тема, действительно, не простая, особенно для новичков, но не такая уж и неразрешимая.
Именно эту проблему и решают YAGNI и Evolutionary Design — достижение наилучшей экономики разработки в условиях недостаточной информированности.
Половина костылей и легаси рождаются именно под девизом
А поэтому, если решение ухудшает экономику разработки, то это не YAGNI, так как оно противоречит его назначению. Например, нарушение Open–closed principle не есть YAGNI. Хороший архитектор, наоборот, максимизирует количество неприятных решений.
Только не очень просто оценить
Воздержусь от обсуждений, выходящих за пределы темы статьи. Оценивание — это тема, действительно, не простая, особенно для новичков, но не такая уж и неразрешимая.
Зато другая половина — из противоположных идей: «у нас эти вещи рассчитываются независимыми компонентами, связи между ними, согласно фен-шую — никакой, но работать как-то надо… а потому давайте-ка заведём тут, посреди этого всего, глобал и будем общаться через него».
связи между ними, согласно фен-шую — никакой, но работать как-то надо… а потому давайте-ка заведём тут, посреди этого всего, глобал и будем общаться через него».
В этом случае нужно просто правильно организовать внедрение зависимостей и соблюдать Low Copling and High Cohesion principle.
Наука, отвечающая за линейный рост стоимости изменения кода, в зависимости от увеличения его объема, называется архитектура. Конечно, если в команде нет опытного архитектора, то никакой CR не поможет.
Конечно, если в команде нет опытного архитектора, то никакой CR не поможет.Знаете, на практике я больше всего страдал от наличия «опытного архитектора».
Потому что в результате произнесения многочисленных акронимов получался код в 10 раз большего объёма, чем до введения «правильной» архитектуры, а главное — он позволял вносить массу бессмысленных измнений и не позволял решать те задачи, которые были нужно решать…
— Зачем нужно городить тут это все, если можно просто сделать?! Кому нужен этот фен-шуй от архитектора?!
Через пару лет…
— Какой урод это написал?! Тут же ничего непонятно! Правишь в одном месте, а падает в другом! Теперь нужно все переписать!
— По истории это написал ты :)
— Я? :/
Через пару лет…
— Какой урод это написал?! Тут же ничего непонятно! Правишь в одном месте, а падает в другом! Теперь нужно все переписать!
— По истории это написал ты :)
— Я? :/
Я эту байку слышу уже больше 10 лет… но почему-то в моём случае она не воспроизводится.
Вот ситатуация «какой &%& вместо переменной A использовал переменную B» (банальная опечатка) — да, бывает. И довольно часто.
А вот с архитектурой… нет. Если мне что-то в когде непонятно — то это точно не мой код. Либо мой, но отрефакторенный «настоящим архитектором» до неузнаваемости (часто ещё и до неработоспособности).
Правда проекты, которые я писал в школе больше 10 лет назад мне поддерживать не приходилось… но с кодом написанным 5-7 лет назад сталкиваюсь регулярно.
Вот ситатуация «какой &%& вместо переменной A использовал переменную B» (банальная опечатка) — да, бывает. И довольно часто.
А вот с архитектурой… нет. Если мне что-то в когде непонятно — то это точно не мой код. Либо мой, но отрефакторенный «настоящим архитектором» до неузнаваемости (часто ещё и до неработоспособности).
Правда проекты, которые я писал в школе больше 10 лет назад мне поддерживать не приходилось… но с кодом написанным 5-7 лет назад сталкиваюсь регулярно.
я больше всего страдал от наличия «опытного архитектора»Не знаю что сказать… Хороший архитектор должен привносить, как раз, облегчение, а не страдание. Разумеется, в балансе долгосрочных и краткосрочных интересов.
Зато другая половина — из противоположных идей
Если мне что-то в коде непонятноИменно этим архитектор и должен заниматься — формированием коллективного понимания того, как устроена система (об этом еще Рольф Джонсон говорил). Если в первом, описанном Вами, случае проблема решалась через глобальный божественный объект, а во втором случае — в 10 раз больше кода, и никто (я полагаю, что не Вы один) не понимает зачем, то, мне трудно судить насколько опытным был ваш архитектор.
Но я хотел бы обратить Ваше внимание на другой момент. Из Ваших слов следует, что вы позиционируете свой уровень знаний выше уровня вашего архитектора (раз уж возмущаетесь его решениями). Но загвоздка в том, что если бы это было действительно так, то Вы бы легко и аргументированно, с отсылкой к первоисточникам, доказали бы ему свою правоту в прямом диалоге, а не выражали бы здесь сейчас свое субъективное отношение к нему. В профессиональном поведении принято разбирать проблемы предметно и по сути, а не выражать свое субъективное отношение к личности, тем более, за спиной этой личности (пусть даже и анонимизируя ее). С ваших слов, ситуация похожа просто на широко известный в ИТ-индустрии эффект Даннинга-Крюгера, ибо единственное, что объективно известно с Ваших слов, так это то, Вы имеете более высокое мнение о собственных способностях, чем это свойственно людям, должность которых подразумевает быть более компетентными.
Хороший архитектор должен привносить, как раз, облегчение, а не страдание.Должен бы. Вот только на практике — как раз обратное происходит.
И, кстати, «не все архитекторы одинаково полезны». Я работал и с людьми, способными создать очень неплохую архитектуру. Вот только они, как правило, не обладают достаточным словарным запасом, чтобы её описание наполовину состояло из звучных акронимов.
Если в первом, описанном Вами, случае проблема решалась через глобальный божественный объект, а во втором случае — в 10 раз больше кодаЯ наверное плохо описал. Как раз там, где было в 10 раз меньше кода (и где всё было жутко не
SOLIDно) — никаких глобалов не было. Потому что всё что нужно — было доступно там, где нужно. Там была пара больших классов и дюжина тривиальных, вспомогательных (фактически структур без кода). А вот в развесистой клюкве с кучаей интерфейсов и полудюжиной уровней абстракции — был глобал. Потому что оказалось, что без этого — нужную информацию через 10 уровней абстракции (5 вверх, 5 вниз) с соблюдением «правил феншуя» передать нельзя. А сроки горели — ну и забили костыль посреди всего этого чудного творения.Из Ваших слов следует, что вы позиционируете свой уровень знаний выше уровня вашего архитектора (раз уж возмущаетесь его решениями).Нет, нет и нет. Умением навешивать лапшу на уши и создавать красивые презентации, в которых долго и пространно объяснялось как предложенная архитектура поможет бороздить просторы Большого Театра… я не обладаю.
Спроектировать работающую систему и сделать её (сам или в команде) — это я могу.
А вот замутить большую кашу и тихо свалить (идеал для архитектора — свалить «на взлёте», доведя до релиза, показав, что архитектура реализована и осталось мелочь — собрать отзывы от пользователей и поправить мелкие недоработки… типа тех самых глобалов, с которыми незвестно что делать) — это, увы, не моё.
Но загвоздка в том, что если бы это было действительно так, то Вы бы легко и аргументированно, с отсылкой к первоисточникам, доказали бы ему свою правоту в прямом диалогеEщё раз: умение создать работающую архитектуру и умение «продать» презентацию начальству — это разные умения. Вот совсем разные.
В профессиональном поведении принято разбирать проблемы предметно и по сути, а не выражать свое субъективное отношение к личности, тем более, за спиной этой личности (пусть даже и анонимизируя ее).Во-первых кто сказал, что речь идёт о каком-то одном архитекторе? Во-вторых — кто вам сказал, что это было за спиной? Обсуждение, как правило, было личным. Но ничего не меняло. Начало-то всегда одинаково:
1. Появление настоящего архитевтора.
2. Много шума.
3. Релиз косого и кривого продукта.
А вот дальше — есть вилка. Иногда архитектор отчаливает, а продукт оказывается в не настолько запущенном состоянии, чтобы его нельзя было исправить. А иногда — проще уйти с проекта, чем разгребать вот это вот всё.
С ваших слов, ситуация похожа просто на широко известный в ИТ-индустрии эффект Даннинга-Крюгера, ибо единственное, что объективно известно с Ваших слов, так это то, Вы имеете более высокое мнение о собственных способностях, чем это свойственно людям, должность которых подразумевает быть более компетентными.Причём тут эффект Даннинга-Крюгера? У нас же послезнание есть. Мы знаем не только то, какую лапшу навешивали «настоящие архитекторы», но и чем всё, по итогу, закончилось.
Я видел много проектов, где архитекторы не были вовлечены — и там, как правило всё было довольно плохо изначально, но со временем архитектура выправлялась и вычищалась.
А вот во всех случаях, когда архитектура была изначально спроектирована «по феншую» — кончалось всё либо дикими костылями, либо выкидыванием всего этого «феншуя» нафиг.
Вот типичная ситуация: принимается решение о том, что мы не будем поддерживать протокол на уровне сериализации в сети (на самом деле
pipe) и всё будет обрабатываться предоставляемыми нами библиотеками — асинхронно. Великая идея, на неё много сил при разработке прототипа положили.Приходит время портировать на эту архитектуру настоящий код… и выясняется, что системная библиотека C не приспособлена к тому, чтобы в момент своего запуска что-то там асинхронно получать!
Попыток накостылить решение без отказа от «феншуя» было несколько, даже Роланда МакГрафа привлекли… его вердикт: Frankly, I think referring to this scheme with «duct tape», «bailing wire», or «chewing gum» does a disservice to all three of those fine building materials.
Пришлось «наступать на горло собственной песне» и вводить синхронный режим… только для запуска, конечно! Точно-точно…
Для всего остального его тоже пришлось вводить, потому что программисты отказывались с этим феншуем работать… правда там уже соорудили синхронный режим поверх асинхронного… с жуткой просадкой производительности и прочим…
Вы вот это хотите назвать «облегчением»?
Нет уж. Архитекторы с титулами, по моему опыту, приносят как раз страдание. Потому что они не порождают код. Они порождают дизайн доки, презентации, доклады… что угодно — но не код.
А вот как раз люди, порождающие код и следующие знаменитому:
Talk is cheap. Show me the code.… да, тут обычно всё хорошо… обычно.
Что-то я не пойму. Если архитектор код никогда не писал — то как вообще в результате его деятельности возникли требования к коду уровня "всё будет обрабатываться предоставляемыми нами библиотеками — асинхронно"? И кто вообще сказал, что архитектор не должен код писать?
Кстати, что такое хитрое должна получать стандартная библиотека Си от пользовательского кода в момент запуска?
Если архитектор код никогда не писал — то как вообще в результате его деятельности возникли требования к коду уровня "всё будет обрабатываться предоставляемыми нами библиотеками — асинхронно"?Выделенный архитектор «с регалиями» написал талмуд с требованиями. Который затем начал исполняться. На замечания рядовых членов была получена пурга из кучи красивых терминов и ссылок на разные книжки, которая впечатлила менеджеров. Ну как это обычно и бывает.
Достаточно типичная ситуация.
И кто вообще сказал, что архитектор не должен код писать?Никто не сказал. И, более того, к людям, которые проектируют архитектуру, а потом её реализуют — у меня, как правило, претензий обычно гораздо меньше. Прежде всего потому, что они реже пытаются воплоть её в жизнь «любой ценой».
Кстати, что такое хитрое должна получать стандартная библиотека Си от пользовательского кода в момент запуска?Много чего, на самом деле. Она должна получить аргументы для запуска приложения (и для лоадера), она должна загрузить это приложение (и все библиотеки от от которых оно зависит), она должна создать «окружение» для программы — причём это происходит по разному если у вас программа однопоточная или многопоточная (да-да, GlibC столько старомодна, что не рассчитывает на то, что любая программа при любых условиях использует потоки). Много чего.
Можно ли написать свою собственную системную библиотеку «с блекджеком и шлюхами», которая вначале будет поднимать «многопоточный стаб» и будет становиться «в позу готовности» для общения через RPC? Да, можно. И ещё можно весь существующий код всех существующих компонент переписать с нуля. Всё можно.
Вопрос: сколько времени это займёт и кто будет за это платить?
Ведь теоретически именно за ответ на этот вопрос архитектор получает деньги… и немалые. Однако на практике — очень часто получаем «дизайн в духе Совы»: мыши, станьте ёжиками!
А на вопрос «а как же так-то» традиционный ответ: «Я решаю важные стратегические вопросы! С тактикой разбирайтесь сами!»…
Она должна получить аргументы для запуска приложения (и для лоадера)
Получает она их от ОС без участия прикладного кода.
она должна загрузить это приложение (и все библиотеки от от которых оно зависит)
И это тоже делается без участия прикладного кода, по построению: он же ещё не загружен.
она должна создать «окружение» для программы
Какое именно и где там может появиться прикладной код?
В общем, я всё ещё не понимаю какие именно проблемы могут встретиться в асинхронном коде при запуске стандартной библиотеки.
Не в том случае, когда у вас программа живёт «в песочнице», которая, «по феншую» общается с миром только через предоставленный ей асинхронный канал.Она должна получить аргументы для запуска приложения (и для лоадера)Получает она их от ОС без участия прикладного кода.
Ну вот вы это знаете — а «великий архитектор»… не знал. Или не задумывался.она должна загрузить это приложение (и все библиотеки от от которых оно зависит)И это тоже делается без участия прикладного кода, по построению: он же ещё не загружен.
argc, argv и aux, которые в main надо передавать, однако… разные библиотеки там разное могут захотеть иметь…она должна создать «окружение» для программыКакое именно и где там может появиться прикладной код?
Не в том случае, когда у вас программа живёт «в песочнице», которая, «по феншую» общается с миром только через предоставленный ей асинхронный канал.
А что меняется-то? Вы там точно прикладную программу писали, а не свою ОС?
Ну вот вы это знаете — а «великий архитектор»… не знал. Или не задумывался.
А какая разница знал он об этом или нет?
argc, argv и aux, которые в main надо передавать, однако… разные библиотеки там разное могут захотеть иметь…
Если у вас библиотека решает какие аргументы передаются в main — то у вас в архитектуре полный бардак, и к асинхронности этот бардак не имеет ни малейшего отношения.
А что меняется-то? Вы там точно прикладную программу писали, а не свою ОС?Вопрос на тему «а была ли это ещё прикладная программа или уже своя ОС» я оставлю философам. Там был свой компилятор (на базе clang), своя системная библиотека (на база GlibC) и вот этот вот новейший и крутейший свой API.
Но вообще задача заключалась в том, чтобы можно было позволить пользователям предоставлять свои «модули расширения», которые бы содержали untrusted код и которые мы бы запускали на наших серверах.
Для конечного результата — скорее никакой… однако если «прогрессивная архитектура» потребовала кучи костылей — то это в любом случае вина архитектора… мне так кажется, по крайней мере.Ну вот вы это знаете — а «великий архитектор»… не знал. Или не задумывался.А какая разница знал он об этом или нет?
Привет астронавтам! Спутитесь, пожалуйста, с небес на землю, а? Добро пожаловать в реальный мир.argc, argv и aux, которые в main надо передавать, однако… разные библиотеки там разное могут захотеть иметь…Если у вас библиотека решает какие аргументы передаются в main — то у вас в архитектуре полный бардак, и к асинхронности этот бардак не имеет ни малейшего отношения.
Рассказы про то, что у кого-то там «в архитектуре бардак» наткнулись на простой ответ (вольный пересказ):
Вот видите — в POSIX есть такая штука, как environ. А в ней — куча стандартных переменных. На которые наши библиотеки смотрят в конструкторах глобальных объектов. И на нестандартные тоже. Вот объясните как в вашей системе environ поменять — так и мы начнём думать о том, стоит ли нам вашу систему использовать… а ваши гениальные мысли — можете себе оставить, не возражаем.Самое обидное — про то, что так и будет куча народу (включая меня) говорили изначально. Но если для наших внутренних демок мы создали вот то самое о котором там аккурантно и толерантно высказался Роланд… то заказчики просто поставили вопрос ребром: когда вы дадите нам возможность задавать
environ. Вопрос «а можно ли будет его в принципе изменять» как-то даже особо не поднимался…
Бывает, сроки поджимают. Бывает, что-то закрываем быстрее. Тогда появляется время закрывать техдолг. Это как кредит у банка. В конечном счете переплачиваешь, но если все продумать, профит должен перевесить.
в конце концов бизнес должен понимать
Нахождение баланса между бизнес-интересами и техническими интересами — очень непростая задача, которая трудно достижима на практике даже для опытных ребят (в силу эффектов Даннинга — Крюгера, Confirmation bias и т.п.). Именно для решения этой задачи и служат 4 переменные (Cost, Time, Quality, Scope), над которыми должен быть организован правильный контроль с соблюдением баланса сторон. Подробности, если интересно, можно найти в «Extreme Programming Explained» 1st edition by Kent Beck.
Это нормально работает, проверено на практике.
Если поправить можно за 5 минут, то конечно надо сразу. А если день — то приоритет как раз определяется тем, насколько это вообще надо менять, и стоит ли тратить день на это.
Опять же во многом вопрос — это новая проблема, которую добавил автор, или это проблема в существовавшем до него коде? Если проблема была, то новый таск необходим ещё и для того, чтобы тестеры понимали что надо проверить.
Если поправить можно за 5 минут, то конечно надо сразу. А если день — то приоритет как раз определяется тем, насколько это вообще надо менять, и стоит ли тратить день на это.
Опять же во многом вопрос — это новая проблема, которую добавил автор, или это проблема в существовавшем до него коде? Если проблема была, то новый таск необходим ещё и для того, чтобы тестеры понимали что надо проверить.
Ну мы обсуждаем кодервью(т.е. проблема в новом коде). В результате этого ревью было решение что и «так сойдет». Т.е. приоритет — «менять не надо». К чему вообще таски с таким приоритетом
Ну мы обсуждаем кодервью(т.е. проблема в новом коде).Новый код существует не в вакууме и зависит от уже существующего кода и принятых в этом коде решений. Ваш К.О.
Во-первых не факт. Пример — девелопер написал код с «магическим числом», но там весь фал такой. Делать одну константу — криво, надо всё выносить, а это сильно больше чем начальная задача. Но ведь замечено именно на ревью.
Во-вторых часто бывает, что задачу релизить надо быстро, а отрефакторить можно через 2 дня.
В-третьих, я как ревьювер не люблю рефакторинг смешанный с функциональными изменениями. Одним коммитом рефакторинг, вторым функционал — куда лучше. Отдельными задачами — ещё лучше, в будущем будет куда проще смотреть историю.
В-четвёртых, бывают горе-менеджеры, которые слишком много внимания уделяют KPI estimated vs. spent. Иногда лучше не дразнить, так куда проще задокументировать почему 1 час работы превратился в 3 дня.
Ведь очень часто «надо порефакторить» вылезает неожиданно когда уже закопался в код.
В-пятых, приоритет может быть высоким. По крайней мере у нас заказчик очень хорошо соглашается с тем, что надёжный и понятный код в критической части проекта стоит затрат.
Во-вторых часто бывает, что задачу релизить надо быстро, а отрефакторить можно через 2 дня.
В-третьих, я как ревьювер не люблю рефакторинг смешанный с функциональными изменениями. Одним коммитом рефакторинг, вторым функционал — куда лучше. Отдельными задачами — ещё лучше, в будущем будет куда проще смотреть историю.
В-четвёртых, бывают горе-менеджеры, которые слишком много внимания уделяют KPI estimated vs. spent. Иногда лучше не дразнить, так куда проще задокументировать почему 1 час работы превратился в 3 дня.
Ведь очень часто «надо порефакторить» вылезает неожиданно когда уже закопался в код.
В-пятых, приоритет может быть высоким. По крайней мере у нас заказчик очень хорошо соглашается с тем, что надёжный и понятный код в критической части проекта стоит затрат.
Вы забыли ещё одну вещь: очень часто очень-очень хочется порефакторить — но если вы сделаете это до вашего изменения, то вам ваше изменение придётся заново переделывать.
Всё время, на него потраченное — ушло, считай, впустую. И ради чего? Ради красивой истории проекта?
Вот тут очень хорошо срабатывает Gerrit: пишем в первое изменение «надо бы порефакторить», ждём второго (который поверх первого), ставим LGTM на оба…
Всё время, на него потраченное — ушло, считай, впустую. И ради чего? Ради красивой истории проекта?
Вот тут очень хорошо срабатывает Gerrit: пишем в первое изменение «надо бы порефакторить», ждём второго (который поверх первого), ставим LGTM на оба…
очень-очень хочется
зачем отказывать себе в удовольствиях? :)
А если серьёзно, то самый частый случай таки моё «во-вторых» — когда тестеры нашли проблему за день до релиза, сегодня мы подопрём костылём и точно больше ничего не сломаем, а послезавтра порефакторим и нормально потестим. И понятно, что надо, а то через месяц костыль будет ставить некуда.
Хм. А как это работает — к примеру вы пишете код, передаете мне на ревью. Я пишу вам 3 замечания. Вы на каждое замечание создаете задачу и не исправляя замечаний пушите код в мастер? Какой смысл такого ревью вообще
Откуда берется срок 2 дня?
Откуда берется срок 2 дня?
Так это не работает, конечно. Даже на уровне policy репозитория — ревьювер должен согласится с моей реакцией на комментарий.
Потому я могу предложить создать отдельную задачу, Вы можете согласиться с моими доводами (и тогда дальше оцениваем и создаём задачу) или не согласиться, и тогда я буду править код прямо сейчас. Можно подключить остальную команду и лида к решению.
Срок 2 дня для примера — реальные оценки и приоритеты определяются по установленному в команде процессу. Пример был в том, что полный рефакторинг может занимать больше времени, чем есть до релизу, и в реальном бизнесе релиз с риском ошибки может быть полезнее позднего релиза без ошибки.
Потому я могу предложить создать отдельную задачу, Вы можете согласиться с моими доводами (и тогда дальше оцениваем и создаём задачу) или не согласиться, и тогда я буду править код прямо сейчас. Можно подключить остальную команду и лида к решению.
Срок 2 дня для примера — реальные оценки и приоритеты определяются по установленному в команде процессу. Пример был в том, что полный рефакторинг может занимать больше времени, чем есть до релизу, и в реальном бизнесе релиз с риском ошибки может быть полезнее позднего релиза без ошибки.
В моей практике происходит обычно следующее:
— делаем новый модуль\под-модуль
— точно знаем что на следующий спринт модуль будет допиливаться на базе комментов от заказчика или просто допиливаться т.к. большой таск декомпозировали и решили разбить на несколько спринтов
— есть место в коде, которое можно поправить
— будет поправлено в следующем спринте тем кто будет доделывать\переделывать
— делаем новый модуль\под-модуль
— точно знаем что на следующий спринт модуль будет допиливаться на базе комментов от заказчика или просто допиливаться т.к. большой таск декомпозировали и решили разбить на несколько спринтов
— есть место в коде, которое можно поправить
— будет поправлено в следующем спринте тем кто будет доделывать\переделывать
Да, мы делаем так-же. Т.е. если принимается решение не исправлять замечания по кодервью, то вариантов 2:
— этот модуль как правило может быть вообще мало нужен
— или как вы написали точно будет меняться через неделю.
В обоих случаях таски не нужны
— этот модуль как правило может быть вообще мало нужен
— или как вы написали точно будет меняться через неделю.
В обоих случаях таски не нужны
Для системных компонент это уже не годится, у них длинный цикл разработки, там много неопределенности по пути и необходимости в исследованиях, и иногда нужно хоть как то сделать фичу, а потом когда она заработает, понять как сделать ее как следует.
И часто шераховатости находятся во время ревью. Например когда из за того что дизайн что то не учел, код фичи становится путаным.
Это как раз значит, что нужна такса чтобы понять что прошло не так и потом сделать как надо.
Но уже после того как фича будет обкатана.
Особенно если речь идет про рефакторинг. Иногда задача может цеплять какие-то зависимости, и ты вынужден делать «не очень хорошо», просто потому что это выходит за рамки задачи, и требует отдельного таска.
Перевод оригинальной статьи — https://habr.com/ru/post/467039/
Просто для контекста — change list он у них потому что у них система контроля версий растет от древнего perforce, и там это были именно CL'и (https://www.perforce.com/perforce/doc.051/manuals/p4guide/07_changelists.html)
Интересно что у них нет самого первого (даже нулевого правила) — посмотри требования.
Сколько раз уже видел код, который отлично спроектирован, соответствуем всем гайдлайнам, покрыт тестами, но делает не то что нужно. Или не совсем, не до конца и т.д.
«Идеальность MR» описана ещё в «Совершенном коде», если не ошибаюсь, под названием «правило бойскаута»: оставь поляну чище чем она была когда ты пришел. Абсолютно необходимое правило если у Вас есть легаси (то есть на почти всех реальных проектах).
Сколько раз уже видел код, который отлично спроектирован, соответствуем всем гайдлайнам, покрыт тестами, но делает не то что нужно. Или не совсем, не до конца и т.д.
«Идеальность MR» описана ещё в «Совершенном коде», если не ошибаюсь, под названием «правило бойскаута»: оставь поляну чище чем она была когда ты пришел. Абсолютно необходимое правило если у Вас есть легаси (то есть на почти всех реальных проектах).
Правила бойскаута — хорошо, и стремиться стоит, но тоже без перегибов. Врачебное — не навреди иногда достаточно. Есть время улучшать среду, есть время ставить костыли.
Правила бойскаута — хорошо, и стремиться стоит, но тоже без перегибов.
Ошибка с моей стороны: «не грязнее». Правило именно в том, чтобы поляна не стала грязнее, при этом убрать одну бутылку из существующей горы мусора — уже отлично.
Если кусок кода стал хоть минимально лучше, то уже нельзя блокировать ревью. Можно предложить ещё улучшить, но это уже на усмотрение автора или отдельной задачей.
Самое важное — уважение. Другой человек решил проблему, сделал задачу. Он её сделал так потому что подумал и решил. И если решение работает, оно достойно уважения. Можно рекомендовать ему хорошие подходы. А если он решил проблему плохо, создал новых проблем разработчикам, не подумал, то если это джун, то нужно провести воспитательную беседу, в остальных случаях один раз предупредить а при повторении уволить. Вот и всё. На рынке программистов дефицит, без работы не останется. Нет никакого смысла держать на работе человека который не уважает других членов команды а они его. Почему навязывание внешних признаков хорошего кода к результуту не приводят я писал тут habr.com/ru/post/440414
Сами они забивают на ревью. Приходится юзать их Flutter. То, как написаны сложные компоненты (DataTable, Navigator, SelectableText, ...), — полный шлак и шок. Как они так скатились — неприятное удивление. На десктопах ваще не тестят, такого бездумного и жесткого ad-hoc программинга давно нигде не видел. Все приходится переписывать.
Глядя на android и firebase можно сделать вывод, что гугл совсем не придерживается подобных «хороших» практик.
Оба этих проекта они, ЕМНИП, приобрели с рук, и такой код — отличный пример того, как бывает, если рефакторинг делать "потом".
Никакие «хорошие» практики не сделают из олимпиадника хорошего разработчика.
Давайте поставим вопрос по иному. Допустим, из олимпиадника не сделают. А из кого сделают?
Никакие «хорошие» практики не сделают из олимпиадника хорошего разработчика.
Один из известнейших программистов истории говорил: «I'm not a great programmer; I'm just a good programmer with great habits.». Так что «правильные практики» могут сделать многое. Вопрос в самих практиках.
Умиляет меня это терминология — практики. Какая-то корреляция с чем-то духовным, йогой. Давно пора признать, что программирование это не творчество, а ремесло. А эти практики не какие-то там хорошие привычки, а обязанности. Очень странно будет видеть, скажем, автомеханика, у которого хорошая практика затягивать гайки.
Умиляет меня это терминология — практикиНу, это официальный термин, используемый организацией Agile Alliance, которая берет свое начало с подписания Agile Manifesto. Этот же термин используется в официальном Scrum Guide и Extreme Programming. Как бы мы к нему не относились.
автомеханика, у которого хорошая практика затягивать гайки.Задача практик заключается в том, чтобы все гайки затягивались в нужный момент времени, в нужной последовательности, с нужным усилием (моментом) затяжки, в отведенный интервал времени, выполняли свое назначение и чтобы качество результата не зависело от уровня квалификации конкретного участника команды.
Ну, это официальный термин, используемый организацией Agile Alliance
Agile practices(англ.) — да, но не «практики». В русском языке слово «практика(и)» имеет иное значение и/или коннотацию (ср. «мы поехали на завод на практику», «практика показывает, что...» и т.п.)
Это некорректная калька с английского сродни «мы продаём экспертизу».
practices сущ.
методы работы
техника
устоявшиеся методы
методики
приёмы работы
«Приёмы работы Agile» или что-то подобное.
Ложный друг переводчика as is.
Ну, это официальный термин, используемый организацией Agile Alliance, которая берет свое начало с подписания Agile Manifesto.
С каких пор Agile стал стандартом процесса разработки ПО и может ли он стандартизировать процесс в конкретном реальном бизнесе? То, что шустрые ребята собрали в кучу ряд стандартных подпроцессов и дали им красивые имена не делает эту методу стандартом. Более того в ряде реальных бизнесов и продуктов, Agile не работает в принципе. Каждый реальный бизнес и продукт требует построения своего процесса разработки, основанного на базовых принципах разработки ПО. Вот эти базовые принципы и должен уметь профпригодный разработчик, это не только код писать.
Задача практик заключается в том,… чтобы качество результата не зависело от уровня квалификации конкретного участника команды.
Ну такое возможно, только если работу неквалифицированного сотрудника выполнит кто-то другой.
С каких пор Agile стал стандартом процесса разработки ПО и может ли он стандартизировать процесс
Ну, мы обсуждаем не стандарты, а проблему, озвученную как «Никакие «хорошие» практики не сделают из олимпиадника хорошего разработчика.» Для решения этой проблемы и существуют практики Collaborative Development. И они работают, если, конечно, уметь их «готовить» («Extreme Programming Explained» 1st edition раскрывает эту тему).
Кстати, Agile можно встретить в ряде стандартов, например, в ISO/IEC/IEEE 12207:2017 Systems and software engineering — Software life cycle processes, или в ГОСТ Р 56920-2016.
Что касается корректности перевода термина «практики», то п. 3.3.8 действующего ГОСТ Р ИСО/МЭК 33001-2017 определяет ее как «практика (practice): Определенный тип активности в рамках выполнения процесса.»
шустрые ребята
Искренне надеюсь, что вы ознакомились с фамилиями 17-ти этих «шустрых ребят», и осознаете их вклад в развитие ИТ-индустрии (помимо Agile), прежде чем это написать.
Каждый реальный бизнес и продукт требует построения своего процесса разработки, основанного на базовых принципах разработки ПО.Возможно, но это уже выходит за рамки обсуждаемой проблемы о практиках, которые могли бы сделать из олимпиадника хорошего разработчика.
То, что шустрые ребята собрали в кучу ряд стандартных подпроцессов и дали им красивые имена не делает эту методу стандартом. Более того в ряде реальных бизнесов и продуктов, Agile не работает в принципе.Нужно, наверное, немного внести ясность, что такое Agile и как он возник. Agile — это исторический момент перехода количественных изменений методик разработки в качественные. Примерно как вода, при при накоплении энергии до температуры 100°C, переходит в новое качественное состояние.
Изначально все работали по BDUF, поскольку изменение реализованных проектных решений стоило очень дорого (стоимость росла экспоненциально). BDUF тоже стоит недешево, поскольку он создается в условиях недостаточной информированности (и обработка этой недостаточности требует работы высококвалифицированных архитекторов), а вероятность ошибки в этих условиях слишком высока. Но тем не менее, он стоил дешевле изменения реализованных решений.
В определенный исторический момент уровень зрелости методик разработки (OOP, TDD, Refactoring, Design Patterns, PoEAA и т.п.) достиг такого уровня, при котором изменение уже реализованных проектных решений стало стоить дешевле создания BDUF. График роста стоимости изменения кода изменился с экспоненциального на асимптотический.
Наука, отвечающая за линейный рост стоимости изменения кода, как я уже, говорил, называется архитектура. А поэтому, на собрании, которое организовал всем известный Robert C. Martin (автор Clean Code) для подписания Agile Manifesto в 2001 году, присутствовал ряд ведущих архитекторов того времени: Ward Cunningham, Kent Beck, Martin Fowler и др.
Иными словами, достижение низкой стоимости изменения кода открыло бизнес-возможность вести разработку итеративно, таким образом, чтобы проектные решения будущих итераций опирались на обратную связь (feedback) от практической эксплуатации реализации проектных решений предыдущих итераций.
Отсюда становится понятно, почему бывает что «Agile не работает в принципе», если правильно понимать его принцип (разумеется, кроме принципиальной причины, существует еще и ряд других причин — организационные, финансовые и пр.). Принципиальная же причина заключается в том, что многим проектам не удается удержать стоимость изменения кода низкой (обычно — в силу невысокого внутреннего качества программы), в итоге, исправление реализованных проектных решений стоит настолько дорого, что дешевле спроектировать все заблаговременно.
А поэтому, на собрании, которое организовал всем известный Robert C. Martin (автор Clean Code) для подписания Agile Manifesto в 2001 году,
Спиральный метод разработки, разновидностью которого является Agile, появился не в 2001 году.
Да, если интересны подробности полной истории итеративной разработки.
Спасибо за информацию! Поделюсь своим мнением насчет этих двух подходов, основанном на собственном опыте. На данный момент у меня сложилось четкое убеждение, что оба этих метода разработки в чистом виде — являются крайностями. Минусы BDUF уже проявили себя со временем и мы их знаем, минусы же Agile еще не так очевидны, так как методология достаточно молода.
Одна из самых острых проблем, которую я встречаю в Agile-проектах, это низкое качество продукта, которое выявляется на всех уровнях реализации: от чертовки запутанных требований, не формализованных требований, отсутствие спецификаций до «черт ногу сломит» программного воплощения и феерического внедрения (привет devops). Основной проблемой качества является не само качество кода, как можно подумать изначально, а высокая информационная энтропия системы. Архитектурно сложившаяся энтропия не может быть устранена в принципе никаким рефакторингом, только заменой архитектуры — что подразумевает реализацию с нуля.
Вторая проблема, на которую стоит обратить внимание — это выжимка разработчиков на полную, психологическая нагрузка и стрессы. Конечна она может быть и при BDUF, но часть звеньев этой проблемы встроены в саму методологию Agile.
Ну критика критикой, надо выдвинуть и свою альтернативу. Собственно ничего нового. Методы методами, практики практиками, но не следует слепо следовать манифестам, а принимать решения на основе реальной задачи, ее объема и сроков. Берем лучшее из любой методы, что подходит для работы над конкретным продуктом. Ключ к качеству — не следует избегать первоначального проектирования, тут без специалиста в этой области не обойтись. Не нужна эпик архитектура всего и вся и всех деталей, тут в самом проектировании подключаем итерационный подход. Необходимы формализованные требования, как минимум на 70%. Если их нет, заказчик не знает чего хочет. Если заказчик не знает чего хочет, он не получит качественный продукт, тут он должен это просто понимать.
Одна из самых острых проблем, которую я встречаю в Agile-проектах, это низкое качество продукта, которое выявляется на всех уровнях реализации: от чертовки запутанных требований, не формализованных требований, отсутствие спецификаций до «черт ногу сломит» программного воплощения и феерического внедрения (привет devops). Основной проблемой качества является не само качество кода, как можно подумать изначально, а высокая информационная энтропия системы. Архитектурно сложившаяся энтропия не может быть устранена в принципе никаким рефакторингом, только заменой архитектуры — что подразумевает реализацию с нуля.
Вторая проблема, на которую стоит обратить внимание — это выжимка разработчиков на полную, психологическая нагрузка и стрессы. Конечна она может быть и при BDUF, но часть звеньев этой проблемы встроены в саму методологию Agile.
Ну критика критикой, надо выдвинуть и свою альтернативу. Собственно ничего нового. Методы методами, практики практиками, но не следует слепо следовать манифестам, а принимать решения на основе реальной задачи, ее объема и сроков. Берем лучшее из любой методы, что подходит для работы над конкретным продуктом. Ключ к качеству — не следует избегать первоначального проектирования, тут без специалиста в этой области не обойтись. Не нужна эпик архитектура всего и вся и всех деталей, тут в самом проектировании подключаем итерационный подход. Необходимы формализованные требования, как минимум на 70%. Если их нет, заказчик не знает чего хочет. Если заказчик не знает чего хочет, он не получит качественный продукт, тут он должен это просто понимать.
Боюсь у вас всё ещё маловато опыта.
Полная победа Agile?
Не совсем. Как я сказал: BKL, в конце концов, извели… но весь процесс занял 15 лет — с 1996го по 2011й. А вот Windows NT — имела поддержку SMP и много чего ещё — сразу в момент релиза, после четырёх лет разработки.
То есть Agile не влечёт за собой плохой архитектуры или чего-нибудь подобного… но для этого, во-первых, люди должны активно бороться за хорошую архитектуру, а во-вторых — это занимает в несколько раз дольше, чем в BDUF.
Архитектурно сложившаяся энтропия не может быть устранена в принципе никаким рефакторингом, только заменой архитектуры — что подразумевает реализацию с нуля.Это неправда. Любая проблема может быть устранена постепенным рефакторингом. Мой самый любимый пример — это SMP. Linux был рождён без поддержки SMP. И его разработка заняла, условно, год работы одного человека. В качестве костыля, чтобы «по быстрому» решить проблему был использован подход Giant lockа. Что позволило как-то, криво и косо, но поддерживать SMP ядром, которое для этого нифига не было предназначено — уже в 1996м году. А со временем и его извели.
Полная победа Agile?
Не совсем. Как я сказал: BKL, в конце концов, извели… но весь процесс занял 15 лет — с 1996го по 2011й. А вот Windows NT — имела поддержку SMP и много чего ещё — сразу в момент релиза, после четырёх лет разработки.
То есть Agile не влечёт за собой плохой архитектуры или чего-нибудь подобного… но для этого, во-первых, люди должны активно бороться за хорошую архитектуру, а во-вторых — это занимает в несколько раз дольше, чем в BDUF.
Определенная доля правды в Вашем ответе, действительно, присутствует. Репутация Agile сегодня на рынке, действительно, пострадала. И пострадала она вполне предсказуемо, и здесь масло в огонь подлил Ken Schwaber, когда убедил Jeff Sutherland «оставить инженерные практики за рамками Scrum, чтобы упростить модель и позволить командам брать на себя ответственность за выбор тех или иных практик».
С одной стороны, это облегчило освоение Scrum широкими массами. Это стало модным и массовым. С другой стороны, то, что осваивали широкие массы, на самом деле было немного далековато от принципиальной идеи Agile. Итеративное планирование — это не первопричина, а технически-обоснованная бизнес-возможность, достигаемая в результате Agile-разработки, и без последнего она теряет экономический смысл по сравнению с BDUF. «A steep change cost curve makes XP (Agile) impossible» — Kent Beck (основатель XP, подписант Agile Manifesto, наставник Роберта Мартина).
А по поводу «информационный энтропии», то «хороший архитектор максимизирует количество непринятых решений» — Роберт Мартин. Именно поэтому, уже на следующий год после того, как он в 2001 году организовал собрание для подписания Agile Manifesto, он выпустил книгу «Agile Software Development. Principles, Patterns, and Practices.», которая была посвящена тому, как нужно писать код, чтобы было возможным работать по Agile.
Конечно. Именно об этом говорит Kent Beck: «McConnell writes, „In ten years the pendulum has swung from 'design everything' to 'design nothing.' But the alternative to BDUF [Big Design Up Front] isn’t no design up front, it’s a Little Design Up Front (LDUF) or Enough Design Up Front (ENUF).“ This is a strawman argument. The alternative to designing before implementing is designing after implementing. Some design up-front is necessary, but just enough to get the initial implementation. Further design takes place once the implementation is in place and the real constraints on the design are obvious. Far from 'design nothing,' the XP strategy is 'design always.'»
Ну, и, классическое разъяснение по этому поводу — Is Design Dead?
От себя добавлю, что лично я редко встречал успешный Scrum, и больше предпочитаю Extreme Programming (XP) для небольших команд и SAFe для крупных проектов. Даже если мы и вынуждены работать по Scrum в силу каких-то формальных причин, то всегда стараемся комбинировать его с XP.
С одной стороны, это облегчило освоение Scrum широкими массами. Это стало модным и массовым. С другой стороны, то, что осваивали широкие массы, на самом деле было немного далековато от принципиальной идеи Agile. Итеративное планирование — это не первопричина, а технически-обоснованная бизнес-возможность, достигаемая в результате Agile-разработки, и без последнего она теряет экономический смысл по сравнению с BDUF. «A steep change cost curve makes XP (Agile) impossible» — Kent Beck (основатель XP, подписант Agile Manifesto, наставник Роберта Мартина).
А по поводу «информационный энтропии», то «хороший архитектор максимизирует количество непринятых решений» — Роберт Мартин. Именно поэтому, уже на следующий год после того, как он в 2001 году организовал собрание для подписания Agile Manifesto, он выпустил книгу «Agile Software Development. Principles, Patterns, and Practices.», которая была посвящена тому, как нужно писать код, чтобы было возможным работать по Agile.
Ключ к качеству — не следует избегать первоначального проектирования, тут без специалиста в этой области не обойтись.
Конечно. Именно об этом говорит Kent Beck: «McConnell writes, „In ten years the pendulum has swung from 'design everything' to 'design nothing.' But the alternative to BDUF [Big Design Up Front] isn’t no design up front, it’s a Little Design Up Front (LDUF) or Enough Design Up Front (ENUF).“ This is a strawman argument. The alternative to designing before implementing is designing after implementing. Some design up-front is necessary, but just enough to get the initial implementation. Further design takes place once the implementation is in place and the real constraints on the design are obvious. Far from 'design nothing,' the XP strategy is 'design always.'»
Ну, и, классическое разъяснение по этому поводу — Is Design Dead?
От себя добавлю, что лично я редко встречал успешный Scrum, и больше предпочитаю Extreme Programming (XP) для небольших команд и SAFe для крупных проектов. Даже если мы и вынуждены работать по Scrum в силу каких-то формальных причин, то всегда стараемся комбинировать его с XP.
У вас какие-то комплексы что ли по поводу "олимпиадников"? (которых в Гугле на самом деле немного). Ну и в основном код как минимум неплохой (библиотеки вообще очень хорошие в основном).
Качество библиотек гугла всегда было неплохого качества, не спорю. Но вот что будет дальше большой вопрос. Качество их продуктов падает. Чтобы проверить почту в gmail на современном ПК мне требуется ожидание загрузки в 2-5 секунд, 20МБ! трафика и все это в итоге в тормозном неотзывчивом интерфейсе. В Firefox у меня вообще гугл накрылся медным тазом, не могу никуда залогиниться, на gmail:
Циклическое перенаправление на странице
При соединении с mail.google.com произошла ошибка. Это у тебя олимпиадника хорошего не было. Сарказм если что.
Были разные, но в основной массе они, по моему опыту, на выходе дают дорогостоящие в обслуживании решения. Алгоритмизация да, разработка продукта нет. Был даже один с ученой степенью, эта ученая степень защищала его от необходимости дальнейшего обучения. Разгребая за ним его творчество мы потеряли кучу здоровья и денег. Я не утверждаю, что все олимпиадники такие упертые, я утверждаю, что это звание не должно влиять на решение работать с ним или нет.
Был даже один с ученой степенью, эта ученая степень защищала его от необходимости дальнейшего обучения. Разгребая за ним его творчество мы потеряли кучу здоровья и денег.Есть такое явление в контексте гибкой разработки, здесь вы зря минусуете. Это известная проблема. Еще в конце 90-х Кент Бек говорил, что труднее всего работать по гибким методологиям докторам наук. Во первых, они склонны к заблаговременному поиску решения реализации, что негативно отражается на темпы разработки и чревато ошибкой в условиях недостаточной информированности (т.н. проблема умных людей). А во вторых, «Хороший код выразителен, а не впечатляющ». Этой теме посвящен первый шаг принципов KISS: «Be Humble, don't think of yourself as a super genius, this is your first mistake. By being humble, you will eventually achieve super genius status =), and even if you don't, who cares! your code is stupid simple, so you don't have to be a genius to work with it.»
Но проблема, на самом деле, не в самом уме как таковом, а в их привычке обращаться со своим умом. Для решения этой проблему и существуют «практики», и когда такие ребята осваивают базовые принципы гибкой разработки, — они становится очень успешными (я знаю целый ряд таких примеров, когда умственные способности из тормоза общего процесса превращались в мощнейший его катализатор).
НЛО прилетело и опубликовало эту надпись здесь
К сожалению (или к счастью) многие программисты считают свой код очень личным и любые замечания к нему воспринимают очень болезненно, как будто это часть их тела. Каждый ревью превращается в битву мнений в таком случае.
Совершенно верно. И это довольно существенный пожиратель времени. Чтобы решить эту проблему, мы, в свое время, ввели такое правило: обязательны к устранению только те замечания, которые можно классифицировать хотя бы по одному из существующих каталогов Code Smells. Если ревьюер не может классифицировать замечание, то его оно считается таким же субъективным, как и мнение автора, который имеет такое же право на личное мнение.
Ну, и, кроме того, исчезла «болезненность» замечаний. Одно дело, когда автору противопоставляется мнение его коллеги, а другое дело, когда ему противопоставляется классифицированный Code Smell, задокументированный Робертом Мартином, Мартином Фаулером, или Уордом Кеннингемом. В последнем случае самолюбие не задевается так сильно.
Кроме того, улучшилось понимание цели «рефакторинга» — он должен снижать стоимость развития программы за счет управления сложностью. Если замечание ревьюера не влияет существенно на экономику разработки, то оно считается малозначимым, исходя из чего становится ясной и его приоритетность (т.е. ответ на вопрос «делать или нет» зависит от приоритетности других задач разработчика).
А почему такой Code Smell из каталога не был завернут на автоматической проверке кода, а попал к ревьюверу и украл у него кучу времени?
Вы можете подсказать название статического анализатора, который умеет полноценно распознавать классифицированные Code Smells из описанных каталогов? К примеру, распознавать Shotgun Surgery или Divergent Change? С удовольствием выслушал бы. Известные мне анализаторы умеют распознавать в лучшем случае лишь Feature Envy (не считая проверки Style Guide).
НЛО прилетело и опубликовало эту надпись здесь
Тут есть некоторый конфликт с максимально быстрым порождением отклика.
Потому правило такое: нужно писать что вы успели отревьюить, что не успели — и ожидаются ли дальнейшие замечания. И где.
То есть буквально: «я посмотрел на предлагаемый конфигурационный факл и не понимаю как будет выглядеть такая и сякая ситуация… на код пока не смотрел». Соответственно тот, кто создал изменение — не будет править код и тратить кучу сил на вариант, который тоже может оказаться не окончательным… а ответит своим комментарием.
В противном случае — да, следующее замечание должно уже касаться не всего нового кода, а только того кода, который изменили, чтобы учесть ваши замечания.
Тогда процесс легко и быстро сходится.
Потому правило такое: нужно писать что вы успели отревьюить, что не успели — и ожидаются ли дальнейшие замечания. И где.
То есть буквально: «я посмотрел на предлагаемый конфигурационный факл и не понимаю как будет выглядеть такая и сякая ситуация… на код пока не смотрел». Соответственно тот, кто создал изменение — не будет править код и тратить кучу сил на вариант, который тоже может оказаться не окончательным… а ответит своим комментарием.
В противном случае — да, следующее замечание должно уже касаться не всего нового кода, а только того кода, который изменили, чтобы учесть ваши замечания.
Тогда процесс легко и быстро сходится.
НЛО прилетело и опубликовало эту надпись здесь
Многие ревью системы имеют просто «comments» и окончательное «review submit» что помогает понять на каком этапе ревью.Этой информации недостаточно. Если не сказано явно, какая часть просмотрена, а где ещё возможны замечания, то неясно — куда смотрать, а куда — пока не надо. Если же не реагировать пока не произойдёт «review submit», то нафига все эти промежуточные комментарии кому-то нужны?
НЛО прилетело и опубликовало эту надпись здесь
Сегодня половину прокомментил, завтра остальное.А как я узнаю что именно вы прокомментировали, а что — ещё нет?
Например если вы отревьюили .h файл — можно ли считать, что и .cc файл для соотвествующего класса тоже отревьюен? Или нет?
Или это зависит от желания средней пятки ревьюера и нужно развивать навыки телепатии???
К сожалению в гитхабе это не явно, например.
У нас правило, если нет возможности подняться со стула и сказать лично, то оставлять финальный комментарий «я закончил». Аналогично с другой стороны — если комментариев много, то автор может делать несколько коммитов, и ревьювер должен знать когда можно смотреть.
Тут нужны правила и договорённости.
Лично у меня так: всё ревью должно занимать максимум час (в исключителных случаях я заранее говорю девелоперу что будет дольше, но это показатель того что дизайн и декомпозиция не состоялись). И далее либо очень быстро будут все замечания, либо одно критическое с пометкой «после исправления переревьювить всё» — когда я вижу ожидаю, что исправление поменяет много.
Лично у меня так: всё ревью должно занимать максимум час (в исключителных случаях я заранее говорю девелоперу что будет дольше, но это показатель того что дизайн и декомпозиция не состоялись). И далее либо очень быстро будут все замечания, либо одно критическое с пометкой «после исправления переревьювить всё» — когда я вижу ожидаю, что исправление поменяет много.
К сожалению, есть примеры, когда «разработчик» умудряется делать каждый раз одни и те же ошибки. Он бы у нас даже интервью бы не прошел, но начальство всунуло дешевый ресурс, вот и приходится учить, выдумывать чем загрузить… :(
Я бы про readability добавил
Так есть же
Если ревьювер не понял какой-то момент в коде и просит автора объяснить, что к чему, то лучшим ответом будет изменение кода. Так, чтобы было по коду все было понятно без вопросов.
Readability review это немного другое — это проверка на соответствие стандартам и понятиям языка, адекватно ли используются библиотеки, нет ли велосипедов, соответствует ли именование стандартам языка и гугла и разные другие вещи, которые не определяет линтер.
Добавлю к предыдущему комментатору — www.pullrequest.com/blog/google-code-review-readability-certification.
У гугла есть отдельная категория по ревью — readability, CL нельзя засабмитать пока он не пройдет ревью у человека, к которого есть readability конкретного языка программирования (это что-то вроде внутренней сертификации). Readability reviewer'ы не обращают пристального внимания на функционал и логику, они проверяют соответствие гайдланам и принятому стилю.
Если ты отправляешь много CL'ей по конкретному языку программирования, то readability в этом языке у тебя со временем появится (процесс получения разный для разных языков).
Если у тебя в команде нет людей с нужным readability — есть отдельные группы пользователей с ним, которые на благих началах тебя поревьювят.
У гугла есть отдельная категория по ревью — readability, CL нельзя засабмитать пока он не пройдет ревью у человека, к которого есть readability конкретного языка программирования (это что-то вроде внутренней сертификации). Readability reviewer'ы не обращают пристального внимания на функционал и логику, они проверяют соответствие гайдланам и принятому стилю.
Если ты отправляешь много CL'ей по конкретному языку программирования, то readability в этом языке у тебя со временем появится (процесс получения разный для разных языков).
Если у тебя в команде нет людей с нужным readability — есть отдельные группы пользователей с ним, которые на благих началах тебя поревьювят.
У нас Гугл заказывал описание нашего стека технологий, примеры кода и прочее, чтобы удостоверится, что с нами можно работать, а сами автоматическую проверку кода на стиль ревьюверу отдают, а не Сонару %)
Как-то Сбербанк таким же хвастался на конференции, ан нет, есть кому дно пробивать!
Как-то Сбербанк таким же хвастался на конференции, ан нет, есть кому дно пробивать!
Я даже не знаю — вы недооцениваете или переоцениваете Гугл. Его Style Guide, слава богу, можно посмотреть. И обнаружить, что там есть такие, например, пункты:
Use type deduction only if it makes the code clearer to readers who aren't familiar with the project, or if it makes the code safer. Do not use it merely to avoid the inconvenience of writing an explicit type.
И как ваш Сонар будет проверять выполнение этого правила, а?
P.S. Разумеется автоматически проверяемые вещи отлавливаются и ревьюеру на это смотреть не нужно. Но и правил подобного сорта — достаточно…
Use type deduction only if it makes the code clearer to readers who aren't familiar with the project, or if it makes the code safer. Do not use it merely to avoid the inconvenience of writing an explicit type.
И как ваш Сонар будет проверять выполнение этого правила, а?
P.S. Разумеется автоматически проверяемые вещи отлавливаются и ревьюеру на это смотреть не нужно. Но и правил подобного сорта — достаточно…
Честно говоря, зависит сильно от языка и принятого стиля написания. Читаемость кода, как для стороннего читателя, например, для Java анализаторы довольно хорошо справляются, если изначально следовать тому же Sun Style. Конечно, если стиль «изощренный» или слишком свободный, то на выходе много интересного будет. А если еще используется реактивное программирование, тут, конечно, без живого читателя не обойтись, но и не каждый со стороны сможет понять почему так пишется.
В свое время, лет 50 назад существовали различные ГОСТЫ на создание программных систем.
И целые институты этим занимались, выдавая при этом кучу документации, которой никто не читал. Мы на ВЦ, не ожидая окончания этой процедуры, разработали свою методику создания
и сопровождения программ, основанную на работе «хирургической бригады» их книги Брукса «Мифический человеко-месяц». Эту методику за 40 лет мне удалось несколько развить. Она существенно отличается от того, что пишет Google, и проверена временем, существенно большим, чем время жизни самого Google.
Излагать здесь не буду, ввиду ожидаемого полного неприятия нынешним поколением программистов. Важно то, что она работает, а с ней работаю и я, принадлежащий ко второму поколению советских программистов.
Hint. Из 10 человек, занятых программированием, писать тексты доверяется только одному.
Ну, это всё из Брукса.
И целые институты этим занимались, выдавая при этом кучу документации, которой никто не читал. Мы на ВЦ, не ожидая окончания этой процедуры, разработали свою методику создания
и сопровождения программ, основанную на работе «хирургической бригады» их книги Брукса «Мифический человеко-месяц». Эту методику за 40 лет мне удалось несколько развить. Она существенно отличается от того, что пишет Google, и проверена временем, существенно большим, чем время жизни самого Google.
Излагать здесь не буду, ввиду ожидаемого полного неприятия нынешним поколением программистов. Важно то, что она работает, а с ней работаю и я, принадлежащий ко второму поколению советских программистов.
Hint. Из 10 человек, занятых программированием, писать тексты доверяется только одному.
Ну, это всё из Брукса.
> Излагать здесь не буду, ввиду ожидаемого полного неприятия нынешним поколением программистов.
Блин, звучит странновато. Я круче всех, но ничего не расскажу
Блин, звучит странновато. Я круче всех, но ничего не расскажу
Вон вам сегодняшние госты standartgost.ru/0/757-programmnoe_obespechenie
А про неприятие, ваша карма говорит сама за себя. Похоже что вы не в состоянии формулировать и писать понятные обществу мысли.
А про неприятие, ваша карма говорит сама за себя. Похоже что вы не в состоянии формулировать и писать понятные обществу мысли.
Вот и ещё один отрицательный аспект системы кармы.
У человека могут быть дельные идеи. Но вы не хотите его слушать, потому что «карма плохая».
У человека могут быть дельные идеи. Но вы не хотите его слушать, потому что «карма плохая».
Читая его комментарий, я искренне хотел послушать этого человека. Дело в том что я тоже иногда пропагандирую идеи не принятые сегодняшним большинством, в этом нет ничего странного. Но когда увидел упоминание о карме, начал сомневаться, для этого пошёл в профиль и посмотрел, что же этот человека уже сделал и как это оценено сообществом.
Вы совершенно правы: высказывать и продвигать следует те идеи, которые не приняты большинством. Во всяком случае, пока ты молод. Но и в более зрелом возрасте, если ты поймёшь, что сидишь в колее, полностью всем доволен и тобой довольны все, то нет ли смысла уйти на покой?
Разумеется, речь идет о принципиальных (профессиональных) вопросах.
В житейском смысле программист должен иметь хороший (терпеливый) характер, чтобы успешно взаимодействовать с начальством, коллегами и пользователями.
Большинство (!) программистов такого характера не имеют.
Присутствующие не в счет.
Разумеется, речь идет о принципиальных (профессиональных) вопросах.
В житейском смысле программист должен иметь хороший (терпеливый) характер, чтобы успешно взаимодействовать с начальством, коллегами и пользователями.
Большинство (!) программистов такого характера не имеют.
Присутствующие не в счет.
Вы могли бы хотя бы попробовать донести до нас свет знаний. Глядишь — ваш опыт пригодился бы, или вы узнали более оптимальный. А печалиться огульным нападкам — в интернет не ходить.
«Осчастливить против желания нельзя».
И на всю жизнь не напасешься.
Что касается опыта, то единственный путь его набраться и использовать — это ежедневная работа. Не ради пропитания только, а для самой работы.
Только одержимость работой дает и творческое долголетие и всё остальное.
Вот единственный свет, на который следует идти.
И на всю жизнь не напасешься.
Что касается опыта, то единственный путь его набраться и использовать — это ежедневная работа. Не ради пропитания только, а для самой работы.
Только одержимость работой дает и творческое долголетие и всё остальное.
Вот единственный свет, на который следует идти.
Мне искренне жаль, что при вашей низкой карме вы потратили редкую возможность написать комментарий на бессодержательную общечеловеческую чушь вместо инженерного разговора по делу — ведь вполне может быть, что ваш метод гениален.
Ни один стоящий специалист не боится и не чурается делиться знаниями: для него это уже пройденный этап, и пока другие освоят его материалы, он уже уйдет вперед, сделав при этом отрасль в целом лучше — ведь его коллеги по цеху стали лучше его стараниями.
Напротив, тот, кому любое знание дается с трудом из-за личных качеств, склонен жадничать и бояться конкуренции.
Но это личный выбор каждого.
Ни один стоящий специалист не боится и не чурается делиться знаниями: для него это уже пройденный этап, и пока другие освоят его материалы, он уже уйдет вперед, сделав при этом отрасль в целом лучше — ведь его коллеги по цеху стали лучше его стараниями.
Напротив, тот, кому любое знание дается с трудом из-за личных качеств, склонен жадничать и бояться конкуренции.
Но это личный выбор каждого.
Слова «общечеловеческий» и «чушь» никак не сочетаются.
Общечеловеческие: Труд, Любовь, Родители, Родина. И где здесь — «чушь»?
А ведь я Вам именно с самого начала сказал о главном ресурсе программиста: работе, труде.
Но не о той работе — с семи до пяти в офисе.
Нет. Речь идет о работе от рассвета до заката без выходных и отпусков.
Так не бывает? Нет, бывает, но не у всех и не для всех.
Для этого надо быть, стать трудоголиком.
Кроме того, очень важно знать, что каким бы ты ни бы хорошим специалистом — всегда найдётся лучше.
И полезно быть с таким человеком знакомым.
Чтобы хорошо видеть горизонт. И не впадать в гордыню.
Мне повезло: в молодости я таких людей знал.
Прерываю дозволенные речи…
Пройдёт 30-40 лет, и сами всё узнаете.
Впрочем, два правила из своего опыта Вам скажу.
1) Никогда не откровенничать по телефону, не ругаться и не выяснять отношений. Абонент на той стороне всегда неизвестен.
2) Никогда в переписке не писать ничего такого, о чем потом будете жалеть. Рукописи не горят. И неизвестно кому ваше послание попадет.
При этом в правило 2 автоматически входит и правило 1.
Общечеловеческие: Труд, Любовь, Родители, Родина. И где здесь — «чушь»?
А ведь я Вам именно с самого начала сказал о главном ресурсе программиста: работе, труде.
Но не о той работе — с семи до пяти в офисе.
Нет. Речь идет о работе от рассвета до заката без выходных и отпусков.
Так не бывает? Нет, бывает, но не у всех и не для всех.
Для этого надо быть, стать трудоголиком.
Кроме того, очень важно знать, что каким бы ты ни бы хорошим специалистом — всегда найдётся лучше.
И полезно быть с таким человеком знакомым.
Чтобы хорошо видеть горизонт. И не впадать в гордыню.
Мне повезло: в молодости я таких людей знал.
Прерываю дозволенные речи…
Пройдёт 30-40 лет, и сами всё узнаете.
Впрочем, два правила из своего опыта Вам скажу.
1) Никогда не откровенничать по телефону, не ругаться и не выяснять отношений. Абонент на той стороне всегда неизвестен.
2) Никогда в переписке не писать ничего такого, о чем потом будете жалеть. Рукописи не горят. И неизвестно кому ваше послание попадет.
При этом в правило 2 автоматически входит и правило 1.
Кроме того, очень важно знать, что каким бы ты ни бы хорошим специалистом — всегда найдётся лучше.
И полезно быть с таким человеком знакомым.
Верно. Но как я сказал выше, «Ни один стоящий специалист не боится и не чурается делиться знаниями». В противном случае ваше знакомство с ним бесполезно. В данном случае получается, что общение с вами — бесполезно, независимо от вашей квалификации.
Больше не могу тратить свое время, добра и удачи.
А он сам не хочет говорить
Знаете есть такой фрик Невзоров, он научил разговаривать лошадь, но только оне её показывать не будет. К тому же вот это:
Часто это значит, что по современным меркам программист из него плоховатый.
Излагать здесь не буду, ввиду ожидаемого полного неприятия нынешним поколением программистов.
Знаете есть такой фрик Невзоров, он научил разговаривать лошадь, но только оне её показывать не будет. К тому же вот это:
я, принадлежащий ко второму поколению советских программистов
Часто это значит, что по современным меркам программист из него плоховатый.
НЛО прилетело и опубликовало эту надпись здесь
С PR у меня иногда возникают 2 проблемы, которые тут не описаны.
1. Комментарий к коду с объяснением почему ТАК делать плохо может быть огромным и общая притензия к коду может быть размазана на 3-4 класса (или 5-10 методов). Написание подробного комментария, иногда, занимает минут 30. Исправление — 15 минут. В последнее время я стал сдаваться и сам переписывал этот код.
2. Есть «синьйор-старожил», который крут в легаси, но пишет откровенный спагетти код на новой системе. Уже как год качество его кода не повышается, а ошибки на уровне джуна. Прошлый его ПР мы за 3 итерации вылизали до приемлимого вида (одну итерацию я сам выпиливал ненужные классы). Позапрошлый, каким-то образом прошёл ревью и там полнейшие «страх и ненависть».
1. Комментарий к коду с объяснением почему ТАК делать плохо может быть огромным и общая притензия к коду может быть размазана на 3-4 класса (или 5-10 методов). Написание подробного комментария, иногда, занимает минут 30. Исправление — 15 минут. В последнее время я стал сдаваться и сам переписывал этот код.
2. Есть «синьйор-старожил», который крут в легаси, но пишет откровенный спагетти код на новой системе. Уже как год качество его кода не повышается, а ошибки на уровне джуна. Прошлый его ПР мы за 3 итерации вылизали до приемлимого вида (одну итерацию я сам выпиливал ненужные классы). Позапрошлый, каким-то образом прошёл ревью и там полнейшие «страх и ненависть».
там полнейшие «страх и ненависть»
Описанная вами проблема всегда возникает при использовании «Collective Ownership» в отрыве от «Collaborative Development». Просто начните использовать практики «Collaborative Development».
Написание подробного комментария, иногда, занимает минут 30.
Воот, а говорят что быстрая печать не нужна :)
У нас в команде принят простой принцип: ревью — это всегда запрос со стороны разработчика к коллеге по принципу «одна голова хорошо, две — лучше».
Основная задача — посмотреть код со стороны, потому что из-за эффекта потока/контекста (замыливается глаз) разработчик может пропустить что-то.
Этот принцип сразу отметает проблему с придирками и спорами: запросивший ревью сам решает, принять замечания, или нет. Если это придирки или пролог к холивору — они просто отметаются.
Поскольку обе стороны находятся на одном уровне иерархии в команде, ввод искусственной иерархии на время ревью (обязательство учитывать замечания) недопустим. Эффект знаком, наверно, многим служившим в армии: бывает так, что сослуживцу, выдвинутому на самую младшую, но командную должность иногда срывает башню от микроскопической, но все же власти над сослуживцами. Никакого положительного эффекта для команды такое не несет.
Концептуальные споры должны быть вынесены на отдельную встречу, семинар, публичное обсуждение. Ты узнал что-то новое, нашел какую-то технику или технологию, которая сделает разработку более эффективной, уменьшит количество ошибок в коде и т.д. — обсуди с коллегами, объясни им преимущества, чтобы все этим пользовались. А не в стиле «надо делать так, так лучше потому что лучше, исправляй, а то не заапрувлю» на код-ревью.
Основная задача — посмотреть код со стороны, потому что из-за эффекта потока/контекста (замыливается глаз) разработчик может пропустить что-то.
Этот принцип сразу отметает проблему с придирками и спорами: запросивший ревью сам решает, принять замечания, или нет. Если это придирки или пролог к холивору — они просто отметаются.
Поскольку обе стороны находятся на одном уровне иерархии в команде, ввод искусственной иерархии на время ревью (обязательство учитывать замечания) недопустим. Эффект знаком, наверно, многим служившим в армии: бывает так, что сослуживцу, выдвинутому на самую младшую, но командную должность иногда срывает башню от микроскопической, но все же власти над сослуживцами. Никакого положительного эффекта для команды такое не несет.
Концептуальные споры должны быть вынесены на отдельную встречу, семинар, публичное обсуждение. Ты узнал что-то новое, нашел какую-то технику или технологию, которая сделает разработку более эффективной, уменьшит количество ошибок в коде и т.д. — обсуди с коллегами, объясни им преимущества, чтобы все этим пользовались. А не в стиле «надо делать так, так лучше потому что лучше, исправляй, а то не заапрувлю» на код-ревью.
В целом вы говорите все правильно и здраво. Но по сути и технически есть фундаментальная проблема
То есть если он вместо переменной А возвращает переменную Б, то он может проигнорировать замечание ревьювера? Мне это кажется не логичным. Подход описанный в начале статьи мне кажется более логичным. Если критика по делу (или есть явные ошибки в коде или в реализации) и на соответствие некоторым принятым командой стандартам, то запросивший ревью обязан их исправить — тут нет никакой свободы. Если же есть некая неопределенность в стандартах, и встает вопрос мнение одного против мнения другого без отсылки к стандартам и практикам, то да запросивший может мягко проигнорировать замечание, чтобы не терять время на бесполезные дебаты.
запросивший ревью сам решает, принять замечания, или нет
То есть если он вместо переменной А возвращает переменную Б, то он может проигнорировать замечание ревьювера? Мне это кажется не логичным. Подход описанный в начале статьи мне кажется более логичным. Если критика по делу (или есть явные ошибки в коде или в реализации) и на соответствие некоторым принятым командой стандартам, то запросивший ревью обязан их исправить — тут нет никакой свободы. Если же есть некая неопределенность в стандартах, и встает вопрос мнение одного против мнения другого без отсылки к стандартам и практикам, то да запросивший может мягко проигнорировать замечание, чтобы не терять время на бесполезные дебаты.
То есть если он вместо переменной А возвращает переменную Б, то он может проигнорировать замечание ревьювера? Мне это кажется не логичным
Естественно, это вырожденный пример, доведение до абсурда: человек не проигнорирует указание на очевидную логическую ошибку.
Но да — он имеет право на это, потому что он отвечает за этот код, а не ревьювер и не команда в целом.
Если критика по делу
Так а кто это решит? Вы исходите все из того же неочевидного посыла, что ревьювер компетентнее (и это верно, если разработчик — новичок, за которым присматривают, или просто разработчик мало знаком с этой частью системы).
Мой посыл — ревьювер не в «потоке» у него не замылен глаз — пмсм, корректнее. Это чем-то похоже на программирование в паре, только второй участвует постфактум. Соответственно, он может заметить ошибку, которая пролетела у разрабатывающего «между глаз», как в вашем примере с переменной. Но принятие решения и ответственность все равно на разработчике: это его код, его зона ответственности.
Теоретически, ревьювер может пойти на конфликт, отследить таск, изменение этого участка и исправить такую ошибку самостоятельно, если он так переживает за качество кода, а разработчик глух и туп. Но я лишь один раз за долгие-долгие годы видел «войну коммитов», когда два человека телепали код туда-сюда пытаясь настоять на своей правоте (причем это касалось скорее холивора «как нам строить всё», а не явной ошибки). Это было лишь единожды и разрешилось административно.
В норме — ты пошлёшь код коллеге либо для поиска очевидных общих ошибок (здесь нужно просто быть не от мира сего, чтобы проигнорировать указание на ошибку «возвращаем А, а надо Б» — такого человека выкинут из команды. А раз не выкинули, то он, по мнению команды, компетентен и адекватен), либо для проверки решения в той области, где ревьювер компетентнее (это — в основном его участок работы/ответственности). Это означает, что ты уже считаешь его мнение значимым и готов к нему прислушиваться — ведь иначе ты вовсе не пошлешь код ему на проверку решения. Ты априори готов слушать его критику решения. Сообразно этой мотивации посылающий будет игнорировать только менее значимые, спорные вопросы. При этом нет элемента подспудного несогласия из-за обязательности и подчиненного положения разработчика как в «традиционной» системе.
В общем, в малой команде с малой текучкой своя специфика.
Lissov
То есть код техлида проходит без исключения
То-есть, у вас есть человек-бутылочное горлышко, через которого проходят все коммиты? ЧуднО.
Так а кто это решит? Вы исходите все из того же неочевидного посыла, что ревьювер компетентнее
Это решат договоренности по стандартам в команде. Все что описано и закреплено в стандартах на разработку — некий чеклист, который не всегда можно статическим анализаторов выловить. Если договорились что копипастить внутри класса в разные методы куски по 10 строк — это плохо, значит это плохо и этого быть не должно. Если договорились не хардкодить строковые константы, а декларировать их и описывать — значит надо описывать и т.д.
А все что касается вкусовщины и оптимизации — это да, можно оставлять на усмотрение кодера.
И нет я не исхожу из того что ревьювер компетентнее, хотя это и улучшает качество и ревью и кода. Но в реальности, даже менее опытный девелопер, вполне может пройтись по чеклисту и найти «дыры» в коде более опытного товарища, особенно если товарищ по каким-то причинам был не собран или спешил.
Это решат договоренности по стандартам в команде
Ясно; в моей парадигме это тоже уже принято как стандарт в виде «узнал что-то новое — обсуди с товарищами, договоритесь о принятии».
Я критиковал не этот подход, а часто встречаемый в обсуждения о код-ревью «прессинг» вида «делай вот так, это best practise — аж Сам XYZ про это статью написал и пофиг, что это не стандарт команды, фиг ты у меня проскочишь».
Но в реальности, даже менее опытный девелопер, вполне может пройтись по чеклисту и найти «дыры» в коде более опытного товарища, особенно если товарищ по каким-то причинам был не собран или спешил.
В таком случае тем более стоит доверить более опытному решать — принимать эти замечания или нет (см. также выше об адекватности).
В общем, мы поняли друг друга, я думаю.
Моя рекомендация из личного опыта — куда лучше работает обязательное ревью на уровне настроек репозитория. То есть код техлида проходит без исключения. Естественный результат — все ревьювят код друг-друга, в том числе парами «посмотри мой PR — ок, я как раз закончил свою задачу, ты пока посмотри мой». Каждый одновременно «с двумя шляпами», не возникает вахтёрства.
Ну и да, нужно просто быть позитивнымы и дружить в команде.
Ну и да, нужно просто быть позитивнымы и дружить в команде.
del
Мы команде себя в шутку называем "клин код наци", чтобы пройти код ревью у меня, нужно быть действительно хорошим программистом.
А хороший программист это какой? Который напишет код не умее чем вы можете понять, не глупее чем то что вы можете придумать и так чтобы вам понравится? Вы проводили ревью алгоритмов которы не в состоянии понять в течении пары дней? Как вы работаете с людьми которые лучше вас разбираются в своей предметной области?
Не бывает такого, чтобы кто-то из команды не понимал алгоритм или другой функционал, который нужно реализовать.
Покер плэннинг длится до тех пор пока каждый участник не будет хорошо понимать обсуждаемый функционал. Затем это запекается в подробной спецификации до написания хоть одной строчки кода. Соответственно к моменту старта имлементации, задача фактически решена участниками и расписана в документации.
В процессе ревью чаще всего уже нет необходимости разбираться как это работает, в основном решаются вопросы, касающиеся соблюдения чистоты кода в соответствии с классическими принципами из методички.
Поэтому не понимаю вашего негатива на мой комментарий.
Покер плэннинг длится до тех пор пока каждый участник не будет хорошо понимать обсуждаемый функционал. Затем это запекается в подробной спецификации до написания хоть одной строчки кода.
ок, с таким подходом да, все будут понимать. Но зачем? В чём ценность? Приведу пример из своей области, я разрабатываю граф движки и игры. И когда кто-то делает алгоритм упаковки атласов, или алгоритм подготовки текстур и других данных для хитрого сжатия, или (что интеснее и сложнее) пишет шейдер с эффектом нельзя заранее сказать что нужно накодить. Это будут кодеры, ты им сказал сюда вот эти строчки — они написали. Я же предпочитаю работать с самостоятельно мыслящими и решающими задачи разработчиками. В принципе люди делятся идеей, типа будем так делать, но никто не разжёвывает всё заранее. Тем более, часто в процессе необходимо поиграть с параметрами, иногда поняв что всё неправильно сменить алгоритм. Никогда нет задачи напиши такой код. Есть задача уменьшить вес ресурсов, или уменьшить потребление памяти и т.п. В случае с шейдером с эффектом нужен художник и программист. Художник делает набросок что бы он хотел, программист делает как смог, обсуждают и постепенно рождается крутой эффект. Никто не знал какой он в точности получится, никто не специфицировал его до пикселя. И когда вся команда нормально так работает, когда Вася который делал шейдер заболеет, Петя вполне сможет его поправить поговорив с художником.
Покер плэннинг длится до тех пор пока каждый участник не будет хорошо понимать обсуждаемый функционал.
Planning poker — это форма коллективной оценки, которая используется для оценки Story (поскольку Story выполняется коллективно). Коллективная оценка тоже имеет свою стоимость, и, поэтому, затраты на нее должны находиться в балансе с бизнес-выгодой от этой оценки (обычно это не более 5% от релиза/итерации/спринта в зависимости от конкретной методологии). Высокая точность оценки и понимание всех деталей реализации на этом этапе не требуется (поскольку затраты на точность оценки растут экспоненциально, а бизнес-выгоды от этой точности — линейно). В некоторых методиках (обычно в двух-итеративных или с Continuous Release), для раннего обнаружения отклонений от плана, кроме коллективной оценки, используется еще и индивидуальная оценка, но уже на уровне конкретных задач (которые выполняются индивидуально), на которые декомпозируется Story.
Ну может он код для запуска ядерного арсенала пишет, где любая ошибка может всю цивизизацию погубить… но как-то сомнительно звучит.
В последнее время такой стиль не очень в моде, уж больно код дорогой получается.
В последнее время такой стиль не очень в моде, уж больно код дорогой получается.
НЛО прилетело и опубликовало эту надпись здесь
Очень интересная статья-спасибо)
Но не хватает в рекомендациях двух моментов:
1) как было сказано выше в комментах-сделаю потом может быть оправдано аргументами
2) проверка оптимизации и быстродействия решения (не только Unit-тесты). Например, по LINQ-запросам к СУБД хорошо бы получить от разработки генерируемые скрипты и их отдельно прогнать на базе и среде, максимально приближенных к продовским реалиям. Таким образом можно сразу понять, где поправить код или как изменить индексы, чтобы система не пострадала.
И это важно сделать до передачи решения на нагрузочное и любое другое тестирование, т е до того, как код попадет в кодовую базу.
Пиши ещё
иногда, если нужно добавить более тщательные компонентные тесты — делаем другой таской. очень помогает.
написал вторую часть статьи: habr.com/ru/post/474334
Зарегистрируйтесь на Хабре, чтобы оставить комментарий
Как проводить Code Review по версии Google