Skip to content

CHANGE: Gate PRs on Validate instead of Publish Dry Run - #2506

Open
K-Tone wants to merge 1 commit into
developfrom
anthony/pr-gate-on-validate
Open

K-Tone wants to merge 1 commit into
developfrom
anthony/pr-gate-on-validate

Conversation

@K-Tone

@K-Tone K-Tone commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Since mid-August, "Publish Dry Run inputsystem" has failed on every PR to develop:

- PVP-40-1 (1 error): Changelog sections are well-formed (US-0039)
  - CHANGELOG.md: line 8: header must match regex: ^\[(?<version>.*)\]( - (?<date>\d{4}-\d{2}-\d{2}))?$

The package tooling team (PVP profile maintainers) confirmed this is expected:

  • upm-pvp publish --dry-run has exactly the same requirements as a real publish. It tells you whether you could publish right now.
  • It doesn't read .yamato/wrench/pvp-exemptions.json. Only the central pets.pvp-catalog profiles apply, and their PVP-40-1 exemption for us is pinned to 1.9.0.
  • A package can't be published with an [Unreleased] changelog section, and that exemption won't be extended.

develop is never in a publishable state, so the dry run can't be a PR gate. Following their recommendation, this PR:

  • Gates PRs on Validate. In Tools/CI/Recipes/Triggers.cs, "All Functional Tests" now depends on the nine Validate - inputsystem - <editor> - <platform> jobs instead of the dry run. They run upm-pvp require "rme PVP-160-1 supported .yamato/wrench/pvp-exemptions.json", so our exemptions apply. They already ran as part of the PR gate through the dry run's dependencies, so the gate loses only the publish-only checks. .yamato/triggers.yml was regenerated with Tools/CI/regenerate.sh.
  • Fixes the changelog header. ## [Unreleased] - yyyy-mm-dd is now ## [Unreleased]. Only the - yyyy-mm-dd placeholder failed the regex.
  • Swaps the exemptions. Removed the PVP-40-1 exemption for that header. Added "PVP-41-1": {"all": true} ("Changelog has no [Unreleased] section"), which the PVP-41-1 docs recommend skipping in CI during development.
  • Documents it in Tools/CI/CI-CHANGES.md.

"Publish Dry Run inputsystem" is still generated and can be run manually, for example before a release or to test CI changes. It will keep failing while the changelog has an [Unreleased] section, which is by design.

Testing status & QA

  • Regenerated the recipes locally. The only change to the generated YAML is in .yamato/triggers.yml: the dry-run dependency is replaced by the nine Validate jobs, the same set the dry run depends on. No YAM002 "dependency references a job that does not exist" warnings.
  • The Validate jobs on this PR should pass with the new header and the PVP-41-1 exemption. The dry run will still fail if run manually, as expected.

Overall Product Risks

  • Complexity: Low. CI configuration and changelog header only, no package code.
  • Halo Effect: Low. It only affects the PR gate. The release process changes slightly, see below.

Comments to reviewers

  • Release process: releasing still replaces ## [Unreleased] with ## [x.y.z] - yyyy-mm-dd. The "Bump version" PR after a release must now add back ## [Unreleased] without the - yyyy-mm-dd placeholder, because PVP-40-1 is no longer exempted for it.
  • Job ids: Wrench's ValidationJobs recipe is internal, so the Validate job ids are built in the cookbook the same way Wrench names them. If Wrench changes the naming, regeneration logs a YAM002 warning.
  • I didn't add Preview APV to the gate. It wasn't gated before, and it pulls in dependant packages, some of which we already ignore as unstable. Happy to add it if we want a stricter gate.

Checklist

Before review:

  • Changelog entry added. Not applicable, CI-only change. The changelog header format is changed.
  • Tests added/changed, if applicable. Not applicable.
  • Docs for new/changed API's. Not applicable.

During merge:

  • Commit message for squash-merge is prefixed with CHANGE: ___.

🤖 Generated with Claude Code

@u-pr u-pr Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Harness Review

Ship it

The change moves PR gating to the existing validation jobs and aligns the changelog header with the updated exemptions. I examined all changed files, dependency coverage, regeneration inputs, and changelog readers; no actionable issues were found.

Reviewed commit 9f0fb43

🤖 Helpful? 👍/👎

@codecov-github-com

codecov-github-com Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

@@             Coverage Diff             @@
##           develop    #2506      +/-   ##
===========================================
+ Coverage    78.96%   79.21%   +0.24%     
===========================================
  Files          767      769       +2     
  Lines       140949   141263     +314     
===========================================
+ Hits        111305   111898     +593     
+ Misses       29644    29365     -279     
Flag Coverage Δ
inputsystem_MacOS_6000.0 5.31% <ø> (+<0.01%) ⬆️
inputsystem_MacOS_6000.0_project 77.88% <ø> (+0.37%) ⬆️
inputsystem_MacOS_6000.3 5.31% <ø> (+<0.01%) ⬆️
inputsystem_MacOS_6000.3_project 77.88% <ø> (+0.37%) ⬆️
inputsystem_MacOS_6000.6 5.30% <ø> (+<0.01%) ⬆️
inputsystem_MacOS_6000.6_project 77.94% <ø> (+0.37%) ⬆️
inputsystem_Ubuntu_6000.0 5.31% <ø> (+<0.01%) ⬆️
inputsystem_Ubuntu_6000.0_project 77.79% <ø> (+0.37%) ⬆️
inputsystem_Ubuntu_6000.3 5.32% <ø> (+<0.01%) ⬆️
inputsystem_Ubuntu_6000.3_project 77.79% <ø> (+0.37%) ⬆️
inputsystem_Ubuntu_6000.6 5.30% <ø> (+<0.01%) ⬆️
inputsystem_Ubuntu_6000.6_project 77.85% <ø> (+0.37%) ⬆️
inputsystem_Windows_6000.0 5.31% <ø> (+<0.01%) ⬆️
inputsystem_Windows_6000.0_project 78.01% <ø> (+0.37%) ⬆️
inputsystem_Windows_6000.3 5.31% <ø> (+<0.01%) ⬆️
inputsystem_Windows_6000.3_project 78.00% <ø> (+0.37%) ⬆️
inputsystem_Windows_6000.6 5.30% <ø> (+<0.01%) ⬆️
inputsystem_Windows_6000.6_project 78.07% <ø> (+0.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 12 files with indirect coverage changes

ℹ️ Need help interpreting these results?

@K-Tone
K-Tone requested a review from LeoUnity November 10, 2026 12:43

@LeoUnity LeoUnity left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Left a small suggestion, but will defer to you on acting on it or not

Comment thread .yamato/wrench/pvp-exemptions.json Resolved
@josepmariapujol-unity

Copy link
Copy Markdown
Collaborator

Aware that running [Yamato] Publish Dry Run inputsystem](https://yamato.ds.unity3d.com/job/79666737?utm_source=github) would result in a failed job, yet wanted to test it out in this PR

"Publish Dry Run inputsystem" has failed on every PR since mid-August with
PVP-40-1 on the "## [Unreleased] - yyyy-mm-dd" changelog header. The package
tooling team confirmed this is expected: a dry run has the same requirements
as a real publish, it doesn't read .yamato/wrench/pvp-exemptions.json, and a
package can't be published with an [Unreleased] section. develop is never
publishable, so the dry run can't be a PR gate.

- "All Functional Tests" now depends on the Validate jobs instead of the dry
  run. They run upm-pvp require with our exemptions file, and were already
  part of the gate through the dry run's dependencies.
- The changelog header is now "## [Unreleased]", which passes PVP-40-1.
- Replaced the PVP-40-1 exemption for the placeholder header with a PVP-41-1
  exemption ("Changelog has no [Unreleased] section"), as the PVP-41-1 docs
  recommend during development.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@K-Tone
K-Tone force-pushed the anthony/pr-gate-on-validate branch from 9f0fb43 to c522e60 Compare November 11, 2026 13:40
@K-Tone
K-Tone requested a review from Pauliusd01 November 11, 2026 13:43

@Pauliusd01 Pauliusd01 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know much about CI so my approval is of low merit but the logic seems sound and running that publish job is required before releasing anyway so it won't be ignored

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants