Рабочий процесс проверки кода, где автор запроса на слияние должен объединиться

16

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

  1. Автор кода отправляет запрос на удаление
  2. Рецензент проверяет код
    • Если рецензент одобряет, он оставляет комментарий в духе «Хорошо выглядит, не стесняйтесь сливаться»
    • Если у рецензента есть проблемы, они оставляют комментарий типа «Пожалуйста, исправьте незначительные проблемы X и Y, затем объедините» (для серьезных изменений вернитесь к шагу 2)
  3. Автор кода вносит изменения в случае необходимости, а затем объединяет его или ее собственный запрос извлечения

У меня есть следующие проблемы:

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

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

Каковы некоторые другие преимущества или недостатки этого рабочего процесса? Распространен ли этот рабочий процесс в других командах разработчиков?

aednichols
источник
5
Вы спрашивали людей в вашей организации, почему они выбрали именно этот рабочий процесс? Они, вероятно, в лучшем положении, чтобы осветить соответствующие достоинства, чем мы. Мы бы просто спекулировали.
Роберт Харви,
1
Что происходит в вашей организации, когда рецензент пишет «Пожалуйста, исправьте основную проблему X»?
Док Браун
8
По моему опыту, лучше всего, чтобы исходный автор выполнял слияние в случае разрешения конфликта слияния. Первоначальный автор, как правило, лучше всего подходит для выяснения путей разрешения конфликта слияния.
17 из 26
Мне было бы любопытно относительно логики здесь. Вы должны спросить своих коллег и написать это как ответ на свой вопрос - здесь может быть очень хороший мыслительный процесс или обоснование. Я просто не могу придумать один быстро.
Томас Оуэнс

Ответы:

21

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

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

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

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

Карл Билефельдт
источник
2
Похоже, там больше разнообразия, чем я себе представлял. Мой опыт работы с автоматизированным тестированием заключался в том, что тесты выполняются на ветви до ее слияния, а не после, что становится предварительным условием слияния. Я также видел непроверенные «мелкие» исправления, в том числе и мои собственные, которые приводили к ошибкам.
Aednichols
2
Тесты часто выполняются как постусловие, а также как предварительное условие. Вполне возможно, что изменения из нескольких ветвей вступят в конфликт неочевидными способами, которые не проявятся как конфликт кода и приведут к провалу тестов. На моем рабочем месте мы требуем, чтобы ветвь была в курсе базовой ветки, а также чтобы все тесты проходили до того, как она станет кандидатом на слияние, и слияние происходит автоматически, если эти два условия выполняются. У нас не всегда было это первое условие - до этого у нас действительно были проблемы, которые нужно было освоить нечасто, несмотря на то, что каждая ветвь проходила индивидуально
Мэтью Шарли
3

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

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

mkcor
источник
2

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

Я хотел бы добавить одно замечание: в некоторых инструментах (мы используем TFS) может быть рабочий элемент, связанный с запросом извлечения.

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

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

9Rune5
источник
1

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

gnasher729
источник