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

Управление проектом

Проект OpenJFX руководится Лидерами проекта и Проверяющими.

Сопредседатель проекта: Кевин Расфорт (ID OpenJDK: kcr; ID GitHub: kevinrushforth)
Сопредседатель проекта: Йохан Вос (ID OpenJDK: jvos; ID GitHub: johanvos)

Проверяющие

Список проверяющих находится в отчете OpenJDK Census.

Обзор

Перед тем как код будет отправлен в репозиторий, он должен пройти проверку. Короткая версия политики проверки кода OpenJFX следующая:* Мы определяем официальную роль "Проверяющего", аналогичную роли в проекте JDK, и описываем обязанности Проверяющих.

• Проверяющие, авторы запросов на вытягивание (PR), а также спонсирующие Коммитеры должны удостовериться в следующих моментах перед интеграцией:
  • Учет всех обратной связи от Проверяющих;
  • Выполнение работы всеми Проверяющими, которым было предоставлено право проверять изменения (или указание того, что больше не требуется);
  • Прохождение достаточного времени (не менее одного рабочего дня), чтобы позволить другим высказаться.
• Политика проверки кода признает различные типы изменений с различными минимальными порогами для проверки:
  • Простые низко-влиятельные исправления: один Проверяющий;
  • Высоко-влиятельные исправления: два Проверяющих;
  • Новые возможности / изменения API: CSR для одобрения изменений, включая одобрение со стороны "лидера"; реализация затем требует двух Проверяющих для кода (как и для других "высоко-влиятельных" исправлений выше).Детали

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

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

1. Роль проверяющего для проекта OpenJFXМы определяем официальную роль "Проверяющего", аналогичную роли в проекте JDK. Проверяющий ответствен за проверку изменений кода и помощь в определении, подходит ли изменение для включения в OpenJFX. Мы ожидаем, что Проверяющие будут чувствовать себя ответственными не только за свой участок работы, но и за качество всей библиотеки JavaFX. Другими словами, роль Проверяющего — это роль хранителя. См. ниже для информации о том, что составляет хорошую проверку. Опытному Committer'у со временем можно присвоить роль Reviewera, обеспечивая высокое качество исправлений и активное участие в проверках кода, что демонстрирует необходимый уровень навыков и понимания для компетентной работы как рецензента. JDK использует порог в 32 значимых вклада. Без намерения слишком снижать этот стандарт, одной из вещей, которую мы можем рассмотреть, является ситуация с Committer'ом, который, скажем, сделал 24 вклада, регулярно участвует в проверках кода и предлагает качественные отзывы. Такой Committer может оказаться таким же хорошим рецензентом (или даже лучше), как тот, кто выполнил 32 вклада, но редко, если вообще когда-либо, предоставляет отзывы по предлагаемым исправлениям ошибок. Это должно рассматриваться как довольно гибкое руководство. Окончательное решение о том, готов ли новый Committer стать Reviewer'ом, принимается Reviewerами и лидерами проекта.### 2. Политика проверки кода

Все проверки кода должны проводиться через запрос на вытягивание (pull request), отправленный против этого репозитория GitHub, openjdk/jfx. Должна существовать запись бага JBS перед тем, как будет проведен обзор pull request. Для информации о том, как отправить pull request, см. CONTRIBUTING.md.

Все исправления должны быть проверены хотя бы одним рецензентом с ролью "Reviewer" (то есть "Рецензент"). У нас есть различные пороги для различных типов изменений. В случае спора относительно того, является ли исправление низкого или высокого влияния, оно считается высокого влияния. То есть мы всегда будем стремиться к качеству, "округляя" до следующего более высокого уровня категории. Контрибьютор может указывать, считает ли он, что его изменения имеют низкое или высокое влияние, но это подтверждается рецензентом. Рецензент либо добавляет комментарий, указывающий, что одиночная проверка достаточна, либо же использует команду Skara /reviewers 2, чтобы запросить второго рецензента (рецензент может запросить больше двух рецензентов в некоторых случаях, где исправление может быть особенно рискованным или затрагивать несколько функциональных областей).Комментарии к отзывам могут быть добавлены прямо в pull request на GitHub или в ответ на автоматически сгенерированное электронное письмо "RFR" (Request for Review). Бот Skara переносит комментарии между ними. Чтобы одобрить pull request, рецензент должен сделать это непосредственно в самом pull request. Для помощи в проверке pull request см. следующую страницу справки GitHub.

Правила проверки запроса на слияние (PR):По умолчанию запрос на слияние считается готовым после того, как его одобрит любой "рецензент". В связи с этим, те, кто имеет роль "рецензента", должны выполнять следующие действия при проверке запроса на слияние до его одобрения:* Определите, требуется ли два ревьювера и необходимость участия представителя службы поддержки клиентов (CSR); введите команду /reviewers 2 или /csr по мере необходимости (учтите, что команда /reviewers 2 требует одобрения от двух ревьюверов в общей сложности, хотя бы один из которых имеет роль "Ревьювер"; если вы действительно считаете, что нужна еще одна проверка со стороны второго "R"евьювера, используйте команду /reviewers 2 reviewers)

* Если вы хотите указать свое одобрение, но все же считаете, что нужны дополнительные ревьюверы, вы можете увеличить количество ревьюверов (например, с 2 до 3)
* Если вы хотите привлечь специалиста по конкретной области для проверки запроса на слияние, укажите это в виде комментария такого типа: `Reviewers: @PERSON1 @PERSON2`; запрошенные ревьюверы могут указать, планируют ли они проверять этот запрос
* Если вы хотите гарантировать возможность вашего личного рассмотрения этого запроса на слияние, добавьте комментарий такого типа: `@PRAUTHOR Подожди моего рассмотрения этого PR`, опционально добавьте любые ваши опасения

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

  • Обычный (не обратный) запрос на слияние должен быть направлен на ветку master практически во всех случаях * Обратный запрос на слияние (который будет иметь метку backport) должен быть направлен на текущую ветку стабилизации; рецензент должен проверить, соответствуют ли исправляемые ошибки критериям текущей фазы стабилизацииВот список вещей, которые следует учитывать при проверке запроса на слияние. Это применимо ко всем, кто выполняет проверку, но особенно к "ревьюверам":*

• Убедитесь, что вы понимаете причины возникновения проблемы и то, как предложенный Pull Request решает её.

• Внимательно рассмотрите риск регресса.

• Внимательно рассмотрите любые вопросы совместимости.

• Проверьте, добавляет ли он, удаляет или модифицирует какие-либо публичные или защищённые API, даже если это происходит неявно (например, публичный метод, который переопределяет защищённый метод, или класс, перемещённый из непубличного в публичный пакет); если да, отметьте необходимость создания запроса на изменение (CSR).

• Сначала сосредоточьтесь на实质性的评论而非格式化的评论。

• Проверьте наличие автоматизированного теста; если его нет, попросите создать один, если это возможно.

• Убедитесь, что Pull Request выполнил тесты GitHub Actions (GHA); если они не выполняются, попросите автора Pull Request включить GHA рабочие процессы; если тест провалился на некоторых платформах, проверьте, является ли это настоящей ошибкой (иногда задача завершается сбоем из-за изменений инфраструктуры GHA или мы видим ложное срабатывание GHA).* Локально протестируйте код, если у вас есть опасения относительно того, работает ли он правильно; полезный совет — объедините последнюю версию основного репозитория в ваш локальный ветвь для проверки перед тестированием.

• Если исходная ветка Pull Request давно не синхронизировалась с основной веткой, или если есть коммиты в верхнем репозитории, отсутствующие в исходной ветке и мешающие Pull Request, рассмотрите возможность попросить автора Pull Request объединить последнюю версию основного репозитория, чтобы получить актуальное выполнение GHA.#### Перед тем как интегрировать или спонсировать запрос на вливание (PR):

Система Skara помечает PR как «готовый» после того, как минимальное количество рецензентов проверят его. Перед тем как интегрировать или спонсировать PR, убедитесь, что выполнены следующие действия:

• Все实质性反馈已得到解决，特别是具有审稿人角色的人员提出的反对意见。
  • Если вы внесли изменения согласно实质性 отзывам рецензента, дождитесь его повторной проверки вашего последнего варианта PR (чтобы удостовериться, что он удовлетворён вашими изменениями).
• Все рецензенты, которым были предоставлены права на проверку, завершили её (или указали, что они могут принять PR без своей проверки). В крайне редких случаях руководитель проекта может пропустить этот шаг.
• PR был помечен как «rfr» (как показывает система Skara) и находится в этом состоянии хотя бы один рабочий день (не менее 24 часов, за исключением выходных и праздников). Это позволяет рецензентам из других часовых поясов иметь достаточно времени для проверки. Этот период начинается со времени, когда система Skara добавила метку «rfr» (например, для PR, который ранее находился в режиме черновика, время начинается после выхода PR из черновика и пометки как «rfr»). В крайне редких случаях (например, при сбоях сборки) рецензент может одобрить интеграцию без ожидания одного дня.

А. Низко-влиятельные исправления ошибок.Это обычно изолированные исправления ошибок с небольшим или отсутствующим влиянием за рамками решения конкретной проблемы; в эту категорию также входят исправления тестов (включая большинство новых тестов), исправления документации и исправления примеров приложений (включая большинство новых примеров).Один (1) "R"ецензент обычно достаточно для принятия таких изменений.

B. Высоко-влиятельные исправления ошибок или запросы на новые возможности.

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

Не менее двух (2) рецензентов должны одобрить такие изменения, один из которых обязательно должен быть "R"ецензентом.

C. Новые функции / Добавление API.

Это включает изменения поведения, добавления к спецификациям FXML или CSS и т. д.

Запросы на новые функции приходят со ответственностью, которая превышает просто предоставление "кода для этой новой функции, пожалуйста примите её". Для даже маленьких функций следует учитывать множество факторов. Большие функции потребуют значительного вклада в области дизайна API, программирования, тестирования, поддерживаемости и т. д. Особенность должна быть обсуждена заранее на списке рассылки openjfx-dev для получения ранней обратной связи по концепции (является ли это особенностью, которую мы вероятно примем) и направлению развития API и/или реализации.Чтобы гарантировать согласованность новых возможностей с остальной частью API и желаемым направлением проекта, требуется CSR (Change Request Specification) для новой возможности, добавления в API или поведенческого изменения. CSR должен быть проверен и одобрен руководителем проекта. В настоящее время этим занимается либо Кевин Рашфорд, либо Йохан Вотс, как указано выше.Обзор реализации следует тем же стандартом «двух рецензентов» для более значимых изменений, как описано в категории B. Два рецензента кода могут включать или не включать руководителя, который проверил CSR. Обзор / одобрение CSR — это независимый шаг от обзора / одобрения изменений кода, хотя они могут проходить параллельно.