Skip to content

chore: PR template and validation updates #3577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 47 additions & 21 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
<!-- 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. Delete bullet points below that don't apply, and update the changelog section as appropriate. -->
## Purpose of this PR
[//]: # (
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.
)

<!-- Add short version of the JIRA ticket to the PR title (e.g. "feat: new shiny feature [MTT-123]") -->
### 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.
- 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

<!-- Uncomment and mark items off with a * if this PR deprecates any API:
### Deprecated API
Expand All @@ -25,12 +22,41 @@
- [ ] The users can understand why this API was removed and what they should use instead.
-->

## Backport
### 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.
)

<!-- If this is a backport:
- Add the following to the PR title: "\[Backport\] ..." .
- Link to the original PR.
If this needs a backport - state this here
If a backport is not needed please provide the reason why.
If the "Backports" section is not present it will lead to a CI test failure.
-->
- 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 :_
- [ ] `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
[//]: # (
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.
)
32 changes: 0 additions & 32 deletions .github/workflows/backport-verification.yml

This file was deleted.

10 changes: 10 additions & 0 deletions .github/workflows/pr-keyword-check.yml
Original file line number Diff line number Diff line change
@@ -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
61 changes: 61 additions & 0 deletions .github/workflows/pr-verification.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# 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 - PR 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 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) {
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);
}
26 changes: 18 additions & 8 deletions .yamato/_triggers.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 for trigger comments of ngo or ignore. For example /ci ngo
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.
Expand All @@ -60,15 +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:
expression: |-
pull_request.comment eq "ngo" AND
NOT pull_request.changes.all match "**/Documentation~/**"
cancel_old_ci: true
pull_requests:
- targets:
only:
- "develop"
- "develop-2.0.0"
- "/release\/.*/"
- drafts: false


# Run all tests on trunk on nightly basis.
# Same subset as pull_request_trigger with addition of mobile/desktop/console tests and webgl builds
Expand Down
2 changes: 2 additions & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ Additional documentation and release notes are available at [Multiplayer Documen

### Changed

### Documentation

## [2.4.4] - 2025-07-07

### Added
Expand Down