Как мне справиться с разногласиями в обзоре кода относительно маловероятного крайнего случая?

Я работаю при запуске робототехники в команде покрытия пути, и после отправки запроса на удаление мой код проверяется.

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

Я приведу пример:

Я потратил целый день на написание тестовых случаев для внесения изменений в алгоритм поиска перехода, который я сделал. Он предложил мне разобраться в малопонятном случае, который крайне маловероятен - на самом деле я не уверен, что это вообще возможно. Код, который я сделал, уже работает во всех наших оригинальных тестовых случаях и некоторых новых, которые я нашел. Код, который я сделал, уже проходит наши 300+ симуляций, которые выполняются каждую ночь. Однако, чтобы разобраться с этим неясным делом, мне понадобилось бы 13 часов, которые можно было бы потратить на улучшение работы робота. Чтобы было ясно, предыдущий алгоритм, который мы использовали до сих пор, также не обрабатывал этот неясный случай, и ни разу, в сгенерированных 40k-отчетах, он никогда не возникал. Мы стартап и нам нужно разработать продукт.

У меня никогда раньше не было обзора кода, и я не уверен, что я слишком аргументирован; я должен просто быть тихим и делать то, что он говорит? Я решил держать голову и просто внести изменения, хотя я категорически не согласен с тем, что это было хорошее использование времени.

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

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

малопонятный случай, который крайне маловероятен - на самом деле я не уверен, что это вообще возможно

Отсутствие непроверенного поведения в коде может быть очень важным. Если фрагмент кода запускается, например, 50 раз в секунду, вероятность «один на миллион» будет происходить примерно каждые 5,5 часов времени выполнения. (В вашем случае шансы кажутся ниже.)

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

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

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

Я могу привести пример углового случая, который никогда не мог произойти, что привело к катастрофе.

Когда Ariane 4 разрабатывался, значения от боковых акселерометров были масштабированы, чтобы соответствовать 16-разрядному целому числу со знаком, и потому что максимально возможный выходной сигнал акселерометров при масштабировании никогда не мог превышать 32767, а минимальное значение никогда не могло опускаться ниже - 32768 «не было необходимости в проверке диапазона». В общем, все входные данные должны проверяться диапазоном перед любым преобразованием, но в этом случае это будет попытка поймать невозможный угловой случай.

Несколько лет спустя Ariane 5 находился в стадии разработки, и код для масштабирования боковых акселерометров был повторно использован с минимальными испытаниями, поскольку он был «проверен в использовании». К сожалению, большая ракета могла ожидать большего бокового ускорения, поэтому акселерометры были модернизированы и могли давать большие 64-битные значения с плавающей запятой .

Эти большие значения «завернуты» в коде преобразования, не помнят , не проверок диапазона, и результаты по первому запуску в 1996 году не были хорошими. Это обошлось компании в миллионы и вызвало большой перерыв в программе.

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

Обратите внимание, что в Ariane 4 это не было ошибкой (поскольку все работало хорошо для всех возможных значений) - это было несоблюдение стандартов. Когда код был повторно использован в другом сценарии, он потерпел катастрофическую неудачу, в то время как если бы диапазон значений был обрезан, он, скорее всего, потерпел бы неудачу, и наличие тестового примера для этого могло бы вызвать проверку значений. Стоит также отметить, что, хотя программисты и тестировщики подверглись некоторой критике со стороны следователей после взрыва, руководство, QA и руководство остались за большинством вины.

осветление

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

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

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

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

Другой эффект, который часто происходит, когда вы обрабатываете дополнительные тестовые случаи, ваш алгоритм обязательно становится более общим, и поэтому вы находите способы упростить его и сделать его более устойчивым для всех ваших тестовых случаев. Это экономит ваше время на техническое обслуживание и устранение неисправностей в будущем.

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

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

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

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

По мере того, как вы испытываете, то, что считается «правильным», часто является дискуссионным, и у каждого свой взгляд на то, что это такое. По моему опыту, ограниченное время и внимание могут также сделать обзоры кода крайне противоречивыми - одна и та же проблема может быть обнаружена или не зависеть от разных разработчиков и в разное время одним и тем же разработчиком. Рецензенты в сознании «что бы улучшить этот код?» часто будут предлагать изменения, которые они не добавят к своим собственным усилиям, естественно.

Как опытный программист (более 15 лет в качестве разработчика), меня часто проверяют программисты с меньшим опытом работы, чем я. Иногда они спрашивают меня об изменениях, с которыми я, мягко говоря, не согласен или считаю неважными. Тем не менее, я все еще делаю эти изменения. Я сражаюсь в битвах за изменения только тогда, когда чувствую, что конечный результат вызовет слабость продукта, когда затраты времени очень высоки или когда «правильная» точка зрения может быть поставлена ​​объективной (например, изменение запрашивается за выигрыш » • не работайте на языке, который мы используем, или тест показывает, что заявленное улучшение производительности не является таковым).

Поэтому я предлагаю тщательно выбирать ваши сражения. Два дня написания тестового примера, которые, по вашему мнению, не нужны, вероятно, не стоят времени / усилий на борьбу. Если вы используете инструмент проверки, такой как запросы на получение GitHub, то, возможно, прокомментируйте там о стоимости / выгоде, которую вы чувствуете, чтобы принять к сведению ваше возражение, но согласиться на выполнение работы. Это считается мягким толчком назад, поэтому рецензент знает, что они достигают границы, и, что более важно, включает ваше обоснование, чтобы такие случаи можно было обострить, если они зашли в тупик. Вы хотите избежать эскалации письменных разногласий - вы не хотите иметь аргумент в стиле интернет-форума по поводу различий в работе - поэтому может быть полезно сначала обсудить проблему и записать достоверное резюме итогов обсуждения,дружеское обсуждение все равно решит для вас обоих).

После мероприятия это хорошая тема для обзора спринта или собраний команды разработчиков и т. Д. Представьте его как можно более нейтрально, например: «В обзоре кода Разработчик А определил этот дополнительный тестовый сценарий, на его написание потребовалось еще два дня, команда считает, что дополнительное покрытие было оправдано этой ценой? " - этот подход работает намного лучше, если вы действительно выполняете работу, поскольку он показывает вам в положительном свете; Вы выполнили работу и просто хотите опросить команду на предмет склонности к риску и развития возможностей.

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

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

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

Поскольку вы, кажется, новичок там, вы можете сделать только одну вещь - проконсультироваться с руководителем группы (или руководителем проекта). 13 часов - это деловое решение; для некоторых фирм / команд много; для некоторых ничего. Это не ваше решение, пока нет.

Если ведущий говорит «накрыть это дело», хорошо; если он говорит "нет, пошли его", хорошо - его решение, его ответственность.

Что касается обзоров кода в целом, расслабьтесь об этом. Возвращать вам задание один или два раза - это совершенно нормально.

Одна вещь, я не думаю, что видел адреса в натуральной форме, хотя это было как бы поднято в ответе @SteveBarnes:

Каковы последствия отказа?

В некоторых полях сбой является ошибкой на веб-странице. А ПК синие экраны и перезагружается.

В других областях это жизнь или смерть - автомобиль за рулем застревает. Лечебный ритм перестает работать. Или в ответе Стива: вещи взрываются, что приводит к потере миллионов долларов.

Существует огромная разница в этих крайностях.

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

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

Ответ на этот вопрос должен дать ответ на вопрос «стоит ли это 13 часов времени компаний». Обратите внимание: я сказал компании время. Они оплачивают счета и в конечном итоге решают, чего это стоит. Ваше руководство должно сказать последнее слово в любом случае.

Может быть, поговорить с человеком, который отвечает за приоритеты работы? При запуске может быть технический директор или владелец продукта. Он может помочь выяснить, нужна ли эта дополнительная работа и почему. Также вы могли бы принести свои заботы во время ежедневных простоев (если они у вас есть).

Если нет четкой ответственности (например, владельца продукта) за планирование работы, попробуйте поговорить с окружающими вас людьми. Позже это может стать проблемой, что все подталкивают продукт в противоположном направлении.

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

Действуй как Коломбо .

Если вы никогда не смотрели Коломбо, это было довольно фантастическое шоу. Коломбо был очень скромным персонажем - большинство людей думали, что он немного сумасшедший и о нем не стоит беспокоиться. Но, проявив скромность и просто попросив людей объяснить, он смог найти своего мужчину.

Я думаю, что это также связано с методом Сократа .

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

В вашем случае у вас есть две идеи, но они имеют в основном одну и ту же цель: сделать код лучше.

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

У вашего коллеги сложилось впечатление, что более осторожное отношение к угловым коробкам более ценно.

Что они видят, чего ты не видишь? Что вы видите, что они не видят? Как младший разработчик, вы на самом деле в прекрасном положении, потому что вы, естественно, должны задавать вопросы. В другом ответе кто-то упоминает, насколько удивительно вероятен угловой случай. Таким образом, вы можете начать с: «Помогите мне понять - у меня сложилось впечатление, что X, Y и Z - что мне не хватает? Почему виджет framboozle? У меня сложилось впечатление, что он будет срываться при неоднозначных обстоятельствах. жезл на самом деле покрывает кисти АНЗА? "

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

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

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

Тем не менее, вы говорите, что вам нужно рычаги в вашей группе. Вот как вы получаете рычаги: просите прощения, не спрашивайте разрешения. Добавьте материал в программу по своему усмотрению (в рамках курса, т. Е. Убедитесь, что он полностью решает проблему, которую хочет начальник ...), и сообщите об этом менеджеру или вашим коллегам по факту. Не спрашивайте их: «Это нормально, если я добавлю функцию X». Скорее, просто добавьте функции, которые вы лично хотите в программе. Если они расстроены из-за новой функции или не согласны с ней, можно удалить ее. Если им это нравится, оставь это.

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

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

Обзор кода служит нескольким целям. Одна из них, о которой вы, очевидно, знаете, это: « Соответствует ли этот код цели? » Другими словами, является ли он функционально правильным; это адекватно проверено; правильно ли комментируются неочевидные части; соответствует ли он условиям проекта?

Другая часть обзора кода - обмен знаниями о системе. Для автора и рецензента это возможность узнать об измененном коде и о том, как он взаимодействует с остальной системой.

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

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

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

Сделав много общих заявлений выше, как это относится к вашей конкретной ситуации? Я надеюсь, что теперь очевидно, что мой совет - ответить на обзор открытыми вопросами и обсудить, какой подход имеет наибольшую ценность. Для вашего примера, где предлагается дополнительный тест, то что-то вроде: «Да, мы можем проверить это; я полагаю, что потребуется <время> для его реализации. Как вы думаете, стоит ли это того?» можно сделать, чтобы гарантировать, что тест не нужен? "

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

Наконец, в качестве общего комментария о значении обзоров кода (и в качестве краткого изложения того, что я сказал выше), мне нравится это утверждение в книге «Сопровождающие средства не масштабировать » Дэниела Веттера :

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

Код всегда может быть лучше.

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

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

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

Поскольку вы задаете этот вопрос, я предполагаю, что вы на самом деле не делаете «проверки кода», а вместо этого создаете запрос на извлечение или другой механизм отправки, чтобы другие могли комментировать недетерминированным способом. Итак, теперь у вас возникли проблемы с управлением и определение готового. Я предполагаю, что ваше руководство нерешительно и на самом деле не понимает процесс и цель проверки кода и, скорее всего, не имеет «определения выполненного» (DOD). Потому что, если бы они сделали это, ваш DOD прямо ответил бы на этот вопрос, и вам не пришлось бы приходить сюда и спрашивать.

Как вы это исправите? Хорошо, попросите вашего менеджера дать вам DOD и сказать вам, если вы всегда должны реализовывать все комментарии. Если рассматриваемый человек является вашим менеджером, то ответ очевиден.

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

На самом деле, это то, как младший практикующий выполняет инструкцию от старшего практикующего, когда младший не может согласиться или оценить это.

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

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

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

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

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

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

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

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

Предложения для рецензентов кода, чтобы повысить деловую полезность вашего рецензирования кода (вы, как OP, должны предложить такое изменение):

• Запишите свои комментарии по типу. "Критическое" / "Must-Do" / "Необязательно" / "Предлагаемые улучшения" / "Приятно иметь" / "Я размышляю".

  Если это кажется слишком CDO / анальным / сложным, по крайней мере, используйте 2 уровня: «Необходимо исправить, чтобы пройти проверку и иметь возможность объединить ваши изменения» / «Все остальные».

Предложения по обработке предложений по проверке кода, которые кажутся менее важными:

• Создайте открытый билет в выбранной вами системе билетов (надеюсь, ваша команда его использует), отслеживая предложение

• Поместите тикет # в качестве комментария к элементу проверки кода, если ваш процесс разрешает ответы на комментарии, такие как Fisheye или обзоры по электронной почте.

• Обратитесь к рецензенту и явно спросите, относится ли этот элемент к типу «должен быть исправлен или не будет объединен / выпущен».

  • Если ответ «Да», но вы не согласны, пусть лицо, ответственное за управление проектом (руководитель проекта, ваш менеджер и т. Д.), Примет решение - представьте несогласие полностью и честно. Дело не в том, кто из вас «прав», а в том, что лучше для проекта, поэтому работа PM / manager.

Теперь обработайте этот тикет как любой другой запрос на разработку.

1. Если после эскалации будет решено срочно, обработайте его как любой срочный запрос разработчика. Деприоретизировать другую работу и работать над этим.

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

Вы не должны передавать это руководству.

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

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

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

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

Очень важно, чтобы вы создали код, удовлетворяющий вашим запросам руководства / управления. Любая другая деталь была бы просто «приятной». Если вы являетесь экспертом (читай: «если вы не являетесь младшим разработчиком») в своей области, то вы «имеете право» решать мелкие проблемы, которые вы обнаружите на этом пути, не консультируясь с вашими руководителями каждый раз.

Если вы считаете, что что-то не так, и вы относительно опытны в своей области, то, скорее всего, вы правы .

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

• Меня попросили сделать X, рецензент кода предлагает также сделать Y, я должен сделать это или я должен перейти к более важным вещам?
• Вы предлагаете мне Y, так что вы можете найти хотя бы один тестовый пример, который бы уловил это поведение, чтобы я мог проверить его? Я считаю, что код не будет вызван.
• Может быть, мы не должны сначала разработать безопасный запасной вариант для раскрытых случаев? Поэтому мы ловим их пораньше и фиксируем на ходу, чтобы сосредоточиться на более важных вещах.
• Хорошо, пока я реализую Y, не могли бы вы написать несколько тестов для этого, чтобы мы выполнили эту задачу раз и навсегда ?
• Меня попросили сделать X, и я думаю, что могу сделать Y, если нет других приоритетов. В следующий раз, почему бы вам не подать запрос на добавление функции вместо того, чтобы оставлять его в качестве комментария к обзору моего кода ? По крайней мере, мы можем услышать дальнейшие мнения других членов команды об этой функции перед ее реализацией (как правило, любая важная вещь должна быть функцией и должна обрабатываться более чем одним человеком; обычно проверка кода должна касаться анализа кода и подходов к решениям; остальные это исправление ошибок и особенности).

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

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

В общем, старайтесь избегать обоих следующих факторов:

• Вы не делаете то, что было запрошено
• Вы заставляете своего рецензента чувствовать себя немым

Если он на самом деле просматривает ваш код, это потому, что руководство доверяет ему больше, чем вам.

Когда во время проверки кода возникают разногласия по поводу области видимости:

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

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

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

1. Примите обратную связь.
2. Прежде чем вносить какие-либо изменения в свой код, постарайтесь создать тест для крайнего случая и посмотрите, сможете ли вы получить тестовый сбой (доказательство того, что проблема существует). Если невозможно создать такой тестовый пример и получить тестовый сбой, вы можете сделать вывод, что крайний случай на самом деле невозможен (хотя я бы не решался сделать такой вывод).
3. Если вы можете получить сбой теста, примените соответствующее изменение кода, чтобы пройти тест.

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

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

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

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

Есть рецензенты кода, которые должны оправдать свое существование, создавая бесполезную работу для других.

Какой из них вам решать, и как обращаться со вторым видом - это больше вопрос для workplace.stackexchange.

Если вы используете scrum, то вопрос заключается в том, выполняет ли ваша работа то, что она должна делать (по-видимому, она делает), и вы можете поместить обработку крайне редкого и, возможно, невозможного случая в отставание, где оно будет иметь приоритет, и если оно идет в спринт, тогда ваш рецензент может свободно взять его и сделать 13 часов работы. Если вы выполняете работу X и, поскольку вы делаете работу X, вы понимаете, что работа Y также нуждается в выполнении, тогда работа Y не становится частью работы X, это ее собственная независимая работа.