Руководство по pull request’ам

Спасибо за вклад в Space Station 14. При отправке pull request’ов (PR) следуйте этим рекомендациям, чтобы ваши pull request’ы было легче проверять и принимать.

Прежде чем начать

• Если вы не знакомы с рабочим процессом Git, прочитайте наше руководство по Git и задавайте столько вопросов, сколько нужно, в #howdoicode.

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

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

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

  • Хотфиксы должны нацеливаться на ветку stable, поэтому для них вам нужно ответвить свою рабочую ветку от stable, а не от master.
  • Вы всё ещё сможете открыть свой PR на ветке master, даже если ответвились от stable, но позже передумали.
  • Однако такая рабочая ветка не покажет никаких невыпущенных изменений, которые были приняты в игру с момента последнего релиза. Вы не увидите, конфликтует ли ваш код с этими изменениями или мешает ли им. Это может стать проблемой, если цель вашего PR будет изменена на ветку master. По этой причине изменения контента или функций должны основываться на ветке master.

Содержание

• Создавайте отдельные PR для изменений функций, исправлений багов и очистки/рефакторинга. Это делает изменения проще для проверки, уменьшает конфликты, а также облегчает их откат, если что-то пойдёт не так.

  • Изменения функций и исправления багов должны быть в отдельных PR.
  • Очистки и «рефакторинг», включая переименование переменных и изменения отступов (например, из-за пространств имён уровня файла), должны быть в отдельном PR.
  • Рефакторинги должны быть в отдельном PR. Сюда входят изменения, затрагивающие значительное число публичных API (полей, методов и т. д.), которые требуют изменений в нескольких системах. Они должны выполняться в отдельном PR от любых изменений контента или исправлений багов.
  • Если вы перемещаете файл в другую папку и/или пространство имён, поместите это в отдельный коммит, когда возможно, чтобы было легче понять, что изменилось в файле, а что было просто перемещено.
  • Изменения маппинга должны быть оформлены отдельным PR для каждой изменённой карты. Это касается даже незначительных изменений.

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

  • Старайтесь разбивать свой PR на более мелкие, когда это имеет смысл. Это значительно облегчает чтение и может привести к более быстрым проверкам. Обычно это также проще для вас, означает, что вы получите более раннюю обратную связь и сможете избежать траты времени на изменения, которые придётся переделывать.

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

Тестирование

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

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

• Предоставляйте скриншоты или видео, демонстрирующие проведённое тестирование. Это также упрощает написание отчётов о прогрессе.

Перед отправкой

• Решите/подтвердите, является ли ваше изменение срочным хотфиксом.

  • Хотфиксы после принятия не ждут следующего двухнедельного релиза.
  • Баги, влияющие на живой сервер, или срочные изменения баланса можно считать хотфиксом.
  • Хотфиксом могут стать только изменения, основанные на ветке stable.
  • Когда вы открываете PR на github, вы можете выбрать, на какую ветку он нацелен, и по умолчанию всегда выбирается ветка master. Для хотфикса нужно вручную выбрать ветку stable.
  • Если вы не уверены, должно ли ваше изменение быть хотфиксом, откройте его на ветку master. Затем спросите в PR, можно ли сделать его хотфиксом.

• Решите, открываете ли вы PR как черновик.

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

• Проверьте свой diff с помощью вкладки предпросмотра кода на GitHub.

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

Заполнение шаблона PR

Вы должны заполнить шаблон pull request при отправке pull request’а на GitHub.

1. Раздел About должен объяснять только то, что делает PR.
2. Раздел Why/Balance должен обосновывать изменения в вашем PR.
   1. Небольшим исправлениям багов не нужно пространное обоснование.
   2. Если pull request посвящён балансу, обоснование должно должным образом объяснить, почему изменение необходимо.
   3. Обоснования, которые объясняют лишь то, что pull request делает, или эффекты изменений, неприемлемы.
   4. Крупные дополнения контента должны соответствовать основным принципам дизайна. Достаточно большие дополнения контента могут требовать проектного документа, детализирующего более широкую цель изменений и то, как они вписываются в текущую игру.
3. Раздел Test Plan должен объяснять, как тестировать изменения.
   1. Опишите, как вы тестировали изменения. Подумайте, как кто-то другой, глядя на этот pull request, сможет убедиться, что он работает как задумано.
   2. Если вы исправляете баг, желательно указать шаги для его воспроизведения, то, как он был сломан раньше, и то, как он исправляется вашим pull request’ом.
4. Раздел Technical Details должен давать высокоуровневый обзор изменений.
   1. Это важнее при исправлениях багов или крупных рефакторингах. Предоставление обзора ваших изменений и принятых технических решений помогает нам проверять изменения; иначе нам приходится самим определять, почему было сделано изменение, что резко увеличивает время рассмотрения.
5. PR должен иметь медиа, когда это применимо.
   1. Изменения, затрагивающие визуальную часть, механику или исправления багов, должны сопровождаться медиа, демонстрирующими изменения мейнтейнерам и сообществу.
   2. Если медиа отсутствует, сомнительно, тестировали ли вы своё исправление.
   3. Медиа обычно не требуется, когда исправление интуитивно очевидно из чтения изменений кода (например, инвертированная булева логика).
6. Раздел Breaking Changes должен быть заполнен, если применимо.
   1. Ломающие изменения возникают, когда изменяется следующее:
      1. Был изменён публичный API. Сюда входит изменение имён полей данных YAML.
      2. Код был перемещён в другое пространство имён или пространство имён было изменено.
      3. ID прототипов были изменены или удалены (даже если ID были мигрированы).
   2. Ломающие изменения должны включать полезные советы о том, как их исправить, если они сложны. Простые указания вроде «Use x namespace», «Use Entity<T> instead», «use RefactoredSystem helpers instead» также приветствуются.
7. Раздел Changelog должен быть заполнен, если применимо.
   1. Дополнительную информацию о том, как заполнять список изменений, см. в разделе «Список изменений» внизу этого документа.

После отправки

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

• Не делайте force push в свою ветку после получения проверки, если только мейнтейнер об этом не попросит. Это приводит к тому, что все проверки отображаются как ‘outdated’, даже если они ещё не были учтены.

Проверки

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

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

Как получить проверки

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

• Мейнтейнеры периодически проверяют открытые PR.

• Если первичная проверка занимает несколько дней, уместно попросить о проверке в #pr-review-request.

Ответ на проверки

• Когда вы отвечаете на проверки, нажмите ‘Resolve conversation’ на GitHub после того, как ваш исправленный код будет отправлен.

• Если у вас есть вопросы по проверкам, оставленным к вашему PR (или, конечно, вопросы по коду в целом), не стесняйтесь попросить разъяснений у проверяющего на GitHub или в Discord либо в #howdoicode.

Список изменений

Записи списка изменений помогают игрокам узнавать о новых функциях или изменениях существующих функций.

Шаблон списка изменений

Шаблон PR на Github содержит следующий список изменений, который можно использовать для форматирования вашей записи, чтобы она автоматически обновлялась в игре:

:cl:
- add: Добавили веселье!
- remove: Убрали веселье!
- tweak: Изменили веселье!
- fix: Починили веселье!

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

Каждая запись является либо add, remove, tweak, либо fix. В каждой категории может быть несколько записей. Они задают значок списка изменений и не отображаются в тексте списка изменений.

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

Категории списка изменений

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

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

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

Список изменений карт

Помещение MAPS: в список изменений поместит все списки изменений ниже него в категорию списка изменений карт вместо основной категории.

:cl:
MAPS:
- add: Добавили веселье!
- remove: Убрали веселье!
- tweak: Изменили веселье!
- fix: Починили веселье!

При написании списков изменений маппинга всегда снабжайте изменения префиксом с названием изменяемой станции. Например:

:cl:
MAPS:
- add: На Meta в северо-восточных техтоннелях добавлена новая арена для лазертага.
- remove: На Bagel удалён терминал ID-карт на эвакуационном КПП СБ.
- tweak: На Box ядро ИИ перенесено в центр станции, под мостик.
- fix: На Fland массив СМЭС переподключён, чтобы правильно заряжаться.

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

Вы также можете использовать «на всех станциях», если изменение применяется ко всем станциям.

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

Например:

:cl:
MAPS:
- remove: На многих станциях удалён генератор аномалий.
- remove: На всех станциях удалён терминал управления грузовым шаттлом.

Списки изменений для добавления или удаления станции можно оформлять гораздо свободнее:

:cl:
MAPS:
- add: Добавлена новая станция RA-12 Spire — станция со средним онлайном, ориентированная на инженерию и Теслу.
- add: Добавлена новая станция Gate — фрагментированная станция с высоким онлайном.
- remove: Станция Core удалена.

Админский список изменений

Помещение ADMIN: в список изменений поместит все списки изменений ниже него в админскую категорию вместо основной категории.

:cl:
ADMIN:
- add: Добавили веселье!
- remove: Убрали веселье!
- tweak: Изменили веселье!
- fix: Починили веселье!

или

:cl:
- add: Добавили веселье!
- remove: Убрали веселье!
- tweak: Изменили веселье!
- fix: Починили веселье!
ADMIN:
- add: Добавили веселье!
- remove: Убрали веселье!
- tweak: Изменили веселье!
- fix: Починили веселье!

Как писать эффективный список изменений

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

При написании записей списка изменений следуйте этим рекомендациям:

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

   • Не очень хорошо: «fixed reflected projectiles dealing stamina damage». Это предложение не начинается с заглавной буквы и не заканчивается точкой.

   • Не очень хорошо: «Wide attacks no longer cost stamina, deal weapon damage». Висячее придаточное после запятой.

   • Не очень хорошо: «There is now more structures that can be made out of web». Пропущена точка в конце предложения, и, поскольку «more structures» стоит во множественном числе, правильное спряжение глагола будет «are». Но всё это предложение можно переработать, используя действительный залог, например: «More structures can now be made out of web»

   • Не очень хорошо: «A craft for cloth consisting of silk». Это не полное предложение.

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

   • Хорошо: «Сервер НИО можно разобрать. Внимание: это сбрасывает все разблокированные технологии, очки и текущую дисциплину». Без записи в списке изменений игроки могут не знать, что серверы НИО теперь можно разбирать. Она также достаточно предупреждает их о потере технологий, чтобы они не были случайно застигнуты врасплох.

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

   • Не очень хорошо: «Изменён спрайт покрытия, чтобы он был чуть менее синим». Та же причина, что и выше.

3. Используйте настоящее время, действительный залог.

   • Не очень хорошо: «Мех ХАМЯК был добавлен».

   • Хорошо: «Робототехник теперь может построить мех ХАМЯК». Переработка этого предложения в действительном залоге сделала его более живым и добавила больше полезных деталей.

   • Не очень хорошо: «Добавлены миски с конфетами для очередей». Кто добавляет?

   • Хорошо: «Миски с конфетами теперь можно найти возле очередей». Подлежащим теперь являются «миски с конфетами». Каждое предложение имеет подлежащее и сказуемое.

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

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

   • Не очень хорошо: «Из-за сокращения бюджета револьвер детектива заменён на что-то более подходящее». Что более подходящее?

   • Хорошо: «Револьвер детектива теперь содержит шумовые патроны вместо боевых».

   • Не очень хорошо: «Синдикат изменил свои цены на акции и избавился от некоторых старых пыльных гарнитур». Непонятно, что изменилось и при чём тут «цены на акции» и «пыльные старые гарнитуры».

   • Хорошо: «Гарнитура Синдиката удалена из аплинка». Ясно объясняет, что было изменено.

   • Не очень хорошо: «Из-за сокращения бюджета Nanotrasen космические ручки больше не поставляются на станцию».

   • Хорошо: «Космические ручки больше недоступны».

5. Избегайте технического жаргона.

   • Не очень хорошо: «Исправлено: микроволновки по умолчанию работают 5 секунд, когда ui показывает «мгновенно»». Что такое ui? Можно улучшить, используя общепринятое сокращение «UI». Кстати: мгновенно теперь действительно мгновенно или просто по умолчанию 5 секунд?

6. Задавайте подходящий тон.

   • Не очень хорошо: «Поверите ли? Только что вышла переработка переработки арахнидов! Подробности в PR».

   • Не очень хорошо: «У арахнидов новые спрайты для being creampied». У creampied есть другое, неудачное значение, которое подрывает профессиональный тон записи в списке изменений.