From 915675607f653c0ef672097661074ff6b6e583c4 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 10:39:24 +0200 Subject: [PATCH 01/14] Added documentation section to the changelog --- com.unity.netcode.gameobjects/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 489af4f446..fe185b51cb 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -25,6 +25,8 @@ Additional documentation and release notes are available at [Multiplayer Documen ### Changed +### Documentation + ## [2.4.4] - 2025-07-07 ### Added From f2e17f51719501548681fbc7bd80f7f3c4d255dc Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 11:05:35 +0200 Subject: [PATCH 02/14] Updated PR template --- .github/pull_request_template.md | 56 +++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index bdf3f3801f..5e6a5583fb 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,22 +1,18 @@ +## Purpose of this PR +[//]: # (Describe the purpose of the PR) - +### Jira ticket +_Link to related jira ticket ([Use the smart commits](https://support.atlassian.com/bitbucket-cloud/docs/use-smart-commits/))_ -## Changelog +### Changelog +[//]: # (updated with all public facing changes - API changes, UI/UX changes, behaviour changes, bug fixes. Remove if not relevant.) - Added: The package whose Changelog should be added to should be in the header. Delete the changelog section entirely if it's not needed. - Fixed: If you update multiple packages, create a new section with a new header for the other package. - Removed/Deprecated/Changed: Each bullet should be prefixed with Added, Fixed, Removed, Deprecated, or Changed to indicate where the entry should go. - -## Testing and Documentation - -- No tests have been added. -- Includes unit tests. -- Includes integration tests. -- No documentation changes or additions were necessary. -- Includes documentation for previously-undocumented public API entry points. -- Includes edits to existing public API documentation. - +- Documentation: Contains significant docs changes +- +### Documentation + + +- No documentation changes or additions were necessary. +- Includes documentation for previously-undocumented public API entry points. +- Includes edits to existing public API documentation. + +## Testing & QA + +### Functional Testing +[//]: # (If checked, List manual tests that have been performed.) +_Manual testing :_ +- [ ] `Manual testing done` + +_Automated tests:_ +- [ ] `Covered by existing automated tests` +- [ ] `Covered by new automated tests` + +_Does the change require QA team to:_ + +- [ ] `Review automated tests`? +- [ ] `Execute manual tests`? + +If any boxes above are checked, please add QA as a PR reviewer. + ## Backport + +[//]: # ( +Replace this block with what this PR does and why. Describe what you'd like reviewers to know, how you applied the engineering principles, and any interesting tradeoffs made. +) ### Jira ticket _Link to related jira ticket ([Use the smart commits](https://support.atlassian.com/bitbucket-cloud/docs/use-smart-commits/))_ @@ -9,10 +10,10 @@ _Link to related jira ticket ([Use the smart commits](https://support.atlassian. [//]: # (updated with all public facing changes - API changes, UI/UX changes, behaviour changes, bug fixes. Remove if not relevant.) - Added: The package whose Changelog should be added to should be in the header. Delete the changelog section entirely if it's not needed. -- Fixed: If you update multiple packages, create a new section with a new header for the other package. +- Fixed: If you update multiple packages, create a new section with a new header for the other package. - Removed/Deprecated/Changed: Each bullet should be prefixed with Added, Fixed, Removed, Deprecated, or Changed to indicate where the entry should go. - Documentation: Contains significant docs changes -- + ### Documentation - +[//]: # ( +This section is REQUIRED and should mention what documentation changes were following the changes in this PR. +We should always evaluate if the changes in this PR require any documentation changes. +) - No documentation changes or additions were necessary. - Includes documentation for previously-undocumented public API entry points. - Includes edits to existing public API documentation. ## Testing & QA - +[//]: # ( +This section is REQUIRED and should describe how the changes were tested and how should they be tested when Playtesting for the release. +It can range from "edge case covered by unit tests" to "manual testing required and new sample was added". +Expectation is that PR creator does some manual testing and provides a summary of it here.) + ### Functional Testing [//]: # (If checked, List manual tests that have been performed.) _Manual testing :_ @@ -54,15 +55,8 @@ _Does the change require QA team to:_ If any boxes above are checked, please add QA as a PR reviewer. ## Backport - - - \ No newline at end of file +[//]: # ( +This section is REQUIRED and should link to the PR that targets other NGO version which is either develop or develop-2.0.0 branch +Add the following to the PR title: "\[Backport\] ..." +If this is not needed, for example feature specific to NGOv2.X, then just mention this fact. +) \ No newline at end of file From 4cb6c80056576f566d88f48127e4c408600958f5 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 13:40:30 +0200 Subject: [PATCH 04/14] Updated backport-verification to a full set of PR verification with more mandatory sections --- .github/workflows/backport-verification.yml | 32 ------------ .github/workflows/pr-verification.yml | 57 +++++++++++++++++++++ 2 files changed, 57 insertions(+), 32 deletions(-) delete mode 100644 .github/workflows/backport-verification.yml create mode 100644 .github/workflows/pr-verification.yml diff --git a/.github/workflows/backport-verification.yml b/.github/workflows/backport-verification.yml deleted file mode 100644 index 4e82b13af6..0000000000 --- a/.github/workflows/backport-verification.yml +++ /dev/null @@ -1,32 +0,0 @@ -# This workflow is designed to verify that the pull request description contains a "## Backport" section, which is important as a reminder to account for backports for anyone that works with NGO repository. -# We have 2 development branches (develop and develop-2.0.0) and we need to ensure that relevant changes are landing in only one or both of them -# If the "##Backport" section is missing, the workflow will fail and block the PR from merging, prompting the developer to add this section. - -# The workflow is configured to run when PR is created as well as when it is edited which also counts simple description edits. - -name: "NGO - Backport Verification" - -on: - pull_request: - types: [opened, edited, synchronize, reopened] - branches: - - develop - - develop-2.0.0 - -jobs: - backport-verification: - runs-on: ubuntu-latest - steps: - - name: Checkout code - uses: actions/checkout@v4 - - - name: Check PR description - uses: actions/github-script@v7 - with: - script: | - const pr = context.payload.pull_request; - const body = pr.body || ''; - - if (!body.includes('## Backport')) { - core.setFailed('PR description must include a "## Backport" section. Please add this section and provide information about this PR backport to develop or develop-2.0.0 branch respectively or explain why backport is not needed.'); - } \ No newline at end of file diff --git a/.github/workflows/pr-verification.yml b/.github/workflows/pr-verification.yml new file mode 100644 index 0000000000..bc66b1c621 --- /dev/null +++ b/.github/workflows/pr-verification.yml @@ -0,0 +1,57 @@ +# This workflow is designed to verify that the pull request description contains a "## Backport" section, which is important as a reminder to account for backports for anyone that works with NGO repository. +# We have 2 development branches (develop and develop-2.0.0) and we need to ensure that relevant changes are landing in only one or both of them +# If the "##Backport" section is missing, the workflow will fail and block the PR from merging, prompting the developer to add this section. + +# The workflow is configured to run when PR is created as well as when it is edited which also counts simple description edits. + +name: "NGO - Backport Verification" + +on: + pull_request: + types: [opened, edited, synchronize, reopened] + branches: + - develop + - develop-2.0.0 + +jobs: + pr-verification: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Check PR description + uses: actions/github-script@v7 + with: + script: | + const pr = context.payload.pull_request; + const body = pr.body || ''; + + // List of mandatory PR sections + const requiredSections = [ + { + header: '## Backport', + description: 'PR description must include a "## Backport" section. Please add this section and provide information about this PR backport to develop or develop-2.0.0 branch respectively or explain why backport is not needed.' + }, + { + header: '## Testing & QA', + description: 'PR description must include a "## Testing & QA" section. Please add this section and provide information about the testing performed for this PR. It can range from adding unit tests to full samples and is needed from QA side to analyze PRs while Playtesting for the release.' + }, + { + header: '### Documentation', + description: 'PR description must include a "### Documentation" section. Please add this section and provide information about the documentation changes made in this PR. It's important to keep the documentation up to date with the code changes.' + } + ]; + + const missing = requiredSections.filter(section => !body.includes(section)); + + if (missing.length > 0) { + const message = [ + 'PR description is missing the following required section(s):', + ...missing.map( + s => `- ${s.header}: ${s.description}` + ), + '\nPlease add them to your PR description.' + ].join('\n'); + core.setFailed(message); + } \ No newline at end of file From 3b7a24f32cb1150282642571c5028d592c7b0b22 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 13:48:36 +0200 Subject: [PATCH 05/14] PR verification comments update --- .github/workflows/pr-verification.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-verification.yml b/.github/workflows/pr-verification.yml index bc66b1c621..35db7a65f2 100644 --- a/.github/workflows/pr-verification.yml +++ b/.github/workflows/pr-verification.yml @@ -1,10 +1,12 @@ -# This workflow is designed to verify that the pull request description contains a "## Backport" section, which is important as a reminder to account for backports for anyone that works with NGO repository. -# We have 2 development branches (develop and develop-2.0.0) and we need to ensure that relevant changes are landing in only one or both of them -# If the "##Backport" section is missing, the workflow will fail and block the PR from merging, prompting the developer to add this section. +# This workflow is designed to verify that the pull request description contains a required sections that are important from quality perspective. +# ## Backport section is important as a reminder to account for backports for anyone that works with NGO repository (to 1.X or 2.X branches respectively). +# ## Testing & QA section is important to ensure that the PR has appropriate testing coverage and is important when QA will evaluate PRs before Playtesting for the release. +# ### Documentation section is important to ensure that the documentation is updated with the changes made in the PR. +# If any of the sections is missing, the workflow will fail and block the PR from merging, prompting the developer to add those sections to the PR description. # The workflow is configured to run when PR is created as well as when it is edited which also counts simple description edits. -name: "NGO - Backport Verification" +name: "NGO - PR Verification" on: pull_request: From ebd6c17edbe9deba18bd4fd646aae115e32f1b07 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 14:09:48 +0200 Subject: [PATCH 06/14] Properly not executing our tests when PR is in a draft state --- .github/workflows/pr-verification.yml | 3 ++- .yamato/_triggers.yml | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-verification.yml b/.github/workflows/pr-verification.yml index 35db7a65f2..474a316065 100644 --- a/.github/workflows/pr-verification.yml +++ b/.github/workflows/pr-verification.yml @@ -30,7 +30,8 @@ jobs: const body = pr.body || ''; // List of mandatory PR sections - const requiredSections = [ + const requiredSections = + [ { header: '## Backport', description: 'PR description must include a "## Backport" section. Please add this section and provide information about this PR backport to develop or develop-2.0.0 branch respectively or explain why backport is not needed.' diff --git a/.yamato/_triggers.yml b/.yamato/_triggers.yml index 0b651dfe66..3650fa3efd 100644 --- a/.yamato/_triggers.yml +++ b/.yamato/_triggers.yml @@ -67,7 +67,7 @@ pull_request_trigger: - "develop" - "develop-2.0.0" - "/release\/.*/" - - drafts: false + drafts: false # Run all tests on trunk on nightly basis. From 00cffa6c71b08433702cc681f20e858b84fdfdd8 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 14:37:24 +0200 Subject: [PATCH 07/14] PR trigger changes to correctly exclude draft PRs and documentation only changes --- .yamato/_triggers.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/.yamato/_triggers.yml b/.yamato/_triggers.yml index 3650fa3efd..8abd5fb510 100644 --- a/.yamato/_triggers.yml +++ b/.yamato/_triggers.yml @@ -61,13 +61,12 @@ pull_request_trigger: - .yamato/cmb-service-standalone-tests.yml#cmb_service_standalone_test_testproject_ubuntu_il2cpp_trunk triggers: cancel_old_ci: true - pull_requests: - - targets: - only: - - "develop" - - "develop-2.0.0" - - "/release\/.*/" - drafts: false + expression: |- + (pull_request.target eq "develop" OR + pull_request.target eq "develop-2.0.0" OR + pull_request.target match "/release\/.*/") AND + NOT pull_request.draft AND + NOT push.changes.all match "**/Documentation~/**" # Run all tests on trunk on nightly basis. From a66f97340244342b3f6872f6f11fe4f835a762c0 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 14:54:02 +0200 Subject: [PATCH 08/14] added comment triggers --- .github/workflows/pr-keyword-check.yml | 10 ++++++++++ .yamato/_triggers.yml | 16 ++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 .github/workflows/pr-keyword-check.yml diff --git a/.github/workflows/pr-keyword-check.yml b/.github/workflows/pr-keyword-check.yml new file mode 100644 index 0000000000..4b58d79741 --- /dev/null +++ b/.github/workflows/pr-keyword-check.yml @@ -0,0 +1,10 @@ +name: PR keyword check +on: + workflow_dispatch: + +# This workflow deliberately does nothing of value. The branch rules require the associated "Check PR issue comments for trigger keywords" job to have run before the PR being merged +jobs: + pr-keyword-check: + runs-on: unity-linux-runner + steps: + - run: ls diff --git a/.yamato/_triggers.yml b/.yamato/_triggers.yml index 8abd5fb510..974a8f30ad 100644 --- a/.yamato/_triggers.yml +++ b/.yamato/_triggers.yml @@ -15,6 +15,9 @@ # Focuses on critical validation paths that we should validate before merging PRs # Cancels previous runs on new commits # Excludes draft PRs +# Excludes running when changes ONLY touching documentation files +# Requires `/ci ngo` or `/ci ignore` comment to trigger the job. This was implemented and explained in https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/pull/3577 + # Nightly: # This test validates same subset as pull_request_trigger with addition of mobile/console tests and webgl builds @@ -38,6 +41,18 @@ #----------------------------------------------------------------------------------- +# Gates the merger of PRs as a pre-requisite. Runs when `/ci ngo` or `/ci ignore` is present as an issue comment +check_pr_issue_comments_for_trigger_keywords: + name: Check PR issue comments for trigger keywords + agent: + type: Unity::github::action + action_source: pr-keyword-check.yml + triggers: + expression: |- + pull_request.comment eq "ngo" OR + pull_request.comment eq "ignore" + + # Run all relevant tasks when a pull request targeting the develop or release branch is created or updated. # In order to have better coverage we run desktop standalone tests with different configurations which allows to mostly cover for different platforms, scripting backends and editor versions. # This job will FIRST run "run_quick_checks" jobs (defined in _run-all.yml) since it's the dependency of project pack jobs which is on the lowest dependency tier. This runs the fastest checks (like PVP or code standards) and ONLY IF those pass it will run the rest of the tests. @@ -62,6 +77,7 @@ pull_request_trigger: triggers: cancel_old_ci: true expression: |- + pull_request.comment eq "ngo" AND (pull_request.target eq "develop" OR pull_request.target eq "develop-2.0.0" OR pull_request.target match "/release\/.*/") AND From 7a71053c0ba456e581af4e490bf9124bb35d9456 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 14:56:40 +0200 Subject: [PATCH 09/14] name change for better decription --- .yamato/_triggers.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.yamato/_triggers.yml b/.yamato/_triggers.yml index 974a8f30ad..d9a7dd3ecb 100644 --- a/.yamato/_triggers.yml +++ b/.yamato/_triggers.yml @@ -43,7 +43,7 @@ # Gates the merger of PRs as a pre-requisite. Runs when `/ci ngo` or `/ci ignore` is present as an issue comment check_pr_issue_comments_for_trigger_keywords: - name: Check PR issue comments for trigger keywords + name: Check PR for trigger comments of ngo or ignore. For example /ci ngo agent: type: Unity::github::action action_source: pr-keyword-check.yml From b54aaa6e28f095e27834f7475641cb8d874f2344 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 15:04:20 +0200 Subject: [PATCH 10/14] pr check correction --- .github/workflows/pr-verification.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-verification.yml b/.github/workflows/pr-verification.yml index 474a316065..750d5d63bc 100644 --- a/.github/workflows/pr-verification.yml +++ b/.github/workflows/pr-verification.yml @@ -46,7 +46,7 @@ jobs: } ]; - const missing = requiredSections.filter(section => !body.includes(section)); + const missing = requiredSections.filter(section => !body.includes(section.header)); if (missing.length > 0) { const message = [ From 0bd1e4e32a223c73401a589f131a5abf4c7f2317 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 15:25:15 +0200 Subject: [PATCH 11/14] pr-verification error fix --- .github/workflows/pr-verification.yml | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/.github/workflows/pr-verification.yml b/.github/workflows/pr-verification.yml index 750d5d63bc..d37a08bd0c 100644 --- a/.github/workflows/pr-verification.yml +++ b/.github/workflows/pr-verification.yml @@ -29,9 +29,8 @@ jobs: const pr = context.payload.pull_request; const body = pr.body || ''; - // List of mandatory PR sections - const requiredSections = - [ + // List of mandatory PR sections + const requiredSections = [ { header: '## Backport', description: 'PR description must include a "## Backport" section. Please add this section and provide information about this PR backport to develop or develop-2.0.0 branch respectively or explain why backport is not needed.' @@ -42,19 +41,21 @@ jobs: }, { header: '### Documentation', - description: 'PR description must include a "### Documentation" section. Please add this section and provide information about the documentation changes made in this PR. It's important to keep the documentation up to date with the code changes.' + description: 'PR description must include a "### Documentation" section. Please add this section and provide information about the documentation changes made in this PR. It is important to keep the documentation up to date with the code changes.' } ]; const missing = requiredSections.filter(section => !body.includes(section.header)); if (missing.length > 0) { - const message = [ - 'PR description is missing the following required section(s):', - ...missing.map( - s => `- ${s.header}: ${s.description}` - ), - '\nPlease add them to your PR description.' - ].join('\n'); + let message = 'PR description is missing the following required section(s):\n'; + + const missingDescriptions = missing.map( + s => `- ${s.header}: ${s.description}` + ); + + message += missingDescriptions.join('\n'); + message += '\n\nPlease add them to your PR description.'; + core.setFailed(message); } \ No newline at end of file From 1df5da95dd7a0af6cc4caef832507955a9e712e1 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 17:06:44 +0200 Subject: [PATCH 12/14] removed Documentation check from trigger --- .yamato/_triggers.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.yamato/_triggers.yml b/.yamato/_triggers.yml index d9a7dd3ecb..29172002ba 100644 --- a/.yamato/_triggers.yml +++ b/.yamato/_triggers.yml @@ -81,9 +81,7 @@ pull_request_trigger: (pull_request.target eq "develop" OR pull_request.target eq "develop-2.0.0" OR pull_request.target match "/release\/.*/") AND - NOT pull_request.draft AND - NOT push.changes.all match "**/Documentation~/**" - + NOT pull_request.draft # Run all tests on trunk on nightly basis. # Same subset as pull_request_trigger with addition of mobile/desktop/console tests and webgl builds From 111e55b3f746d4884c3d6f2ad0e98ff6d2a8b084 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 17:11:25 +0200 Subject: [PATCH 13/14] only comment trigger required for the tests to run --- .yamato/_triggers.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.yamato/_triggers.yml b/.yamato/_triggers.yml index 29172002ba..5415bf6933 100644 --- a/.yamato/_triggers.yml +++ b/.yamato/_triggers.yml @@ -77,11 +77,7 @@ pull_request_trigger: triggers: cancel_old_ci: true expression: |- - pull_request.comment eq "ngo" AND - (pull_request.target eq "develop" OR - pull_request.target eq "develop-2.0.0" OR - pull_request.target match "/release\/.*/") AND - NOT pull_request.draft + pull_request.comment eq "ngo" # Run all tests on trunk on nightly basis. # Same subset as pull_request_trigger with addition of mobile/desktop/console tests and webgl builds From 98f93e943045e354e8cfeba290625b05ae657bf8 Mon Sep 17 00:00:00 2001 From: michalChrobot Date: Mon, 4 Aug 2025 17:13:50 +0200 Subject: [PATCH 14/14] corrected the expression --- .yamato/_triggers.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.yamato/_triggers.yml b/.yamato/_triggers.yml index 5415bf6933..b0e7d3bb82 100644 --- a/.yamato/_triggers.yml +++ b/.yamato/_triggers.yml @@ -75,9 +75,10 @@ pull_request_trigger: - .yamato/desktop-standalone-tests.yml#desktop_standalone_test_testproject_win_il2cpp_6000.0 - .yamato/cmb-service-standalone-tests.yml#cmb_service_standalone_test_testproject_ubuntu_il2cpp_trunk triggers: - cancel_old_ci: true expression: |- - pull_request.comment eq "ngo" + pull_request.comment eq "ngo" AND + NOT pull_request.changes.all match "**/Documentation~/**" + cancel_old_ci: true # Run all tests on trunk on nightly basis. # Same subset as pull_request_trigger with addition of mobile/desktop/console tests and webgl builds