Add a reusable PHPStan workflow and the canonical pre-push hook, PG-4897 - #44
Merged
Conversation
Plugin repos currently carry near-identical copies of both; four rounds of hook fixes each meant twelve pull requests. The workflow also fails when a plugin's hook copy drifts from the canonical one.
Pushing another local branch, or several refs at once, analysed the checked-out branch instead of what was actually pushed.
10 tasks
A slug the derivation regex does not match passed through unchanged and reached rm -rf. Also narrows the README example to pass only the secret the workflow uses instead of secrets: inherit.
…in checkout The name reaches path operations in checkout_matomo.sh, so reject anything but an alphanumeric plugin name up front. Without TARGET_BRANCH the dependent plugins silently stayed on their default branch instead of the caller's base branch.
…tside plugins/ job.workflow_repository/job.workflow_sha make the caller's pin govern the scripts too, instead of floating on main. The hook no longer rejects pushes from standalone clones it cannot check.
snake14
marked this pull request as ready for review
August 4, 2026 02:57
snake14
added a commit
that referenced
this pull request
Aug 4, 2026
Centralizing removes the reason the gate floated on main: an update is now one SHA bump here instead of a change in every plugin repo. The README section will conflict trivially with #44's; whichever merges second rebases the append.
No GitHub context identifies the called workflow's own commit — github.job_workflow_sha evaluates to empty (verified in a live run) — so an explicit input is the only honest pinning mechanism.
lachiebol
previously approved these changes
Aug 4, 2026
lachiebol
left a comment
lachiebol
left a comment
There was a problem hiding this comment.
LGTM, Codex is complaining about a documentation gap, might be good to address.
Medium
-
The dependent-plugin validation rejects a documented repository form. scripts/bash/checkout_dependent_plugins.sh:12 only derives names from owner/plugin-PluginName; README.md:102
promises support for both owner/PluginName and owner/plugin-PluginName. The new validation then silently skips the bare-name form. This breaks the declared input contract and can run
PHPStan without a requested dependency.Parse and validate the complete slug explicitly, supporting both documented forms, for example ^owner/(plugin-)?PluginName$.
Contributor
Author
Thanks @lachiebol . It should now be addressed. |
lachiebol
approved these changes
Aug 4, 2026
AltamashShaikh
approved these changes
Aug 5, 2026
10 tasks
Contributor
Author
|
Can someone from @matomo-org/core-reviewers please merge this and #45 if they look alright? |
10 tasks
Contributor
Author
10 tasks
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Centralizes the PHPStan CI logic that twelve plugin repositories currently carry as near-identical copies. Four rounds of fixes to the shared pre-push hook (ddev fallback,
persist-credentials, merge-base base resolution, diffing pushed commits instead of the working tree) each required a pull request per plugin repo; with this change, such fixes land once here.Two pieces:
.github/workflows/plugin-phpstan.yml— a reusable workflow (workflow_call). A plugin's ownphpstan.ymlshrinks to a ~10-line caller passingplugin-name, and optionallydependent-plugins(e.g. GoogleAnalyticsImporter needs MarketingCampaignsReporting + Funnels) andphp-version(GoogleAnalyticsImporter runs on 8.2 because ConnectAccounts uses PHP 8 attribute syntax). The analysis level andphpVersionstay in each plugin'sphpstan.neon.hooks/pre-push— the canonical copy of the opt-in developer hook (byte-identical to what the twelve open PG-4897 PRs currently ship). Git hooks must exist as local files, so plugins keep their copy; the reusable workflow's newverify-hookstep fails the build when a plugin's copy drifts from the canonical one, replacing silent divergence with a red check.Migration of the twelve plugin repos to the caller form would follow as separate per-repo PRs once this shape is agreed.
Smoke-tested end to end from disposable caller branches: LogViewer (simple path) and GoogleAnalyticsImporter (dependent plugins, private-repo secret, PHP 8.2) both pass. The testing surfaced two fixes now included: minimal callee permissions (a blanket-read callee cannot be called from a default-permission caller at all) and the
scripts-refinput (no GitHub context identifies the called workflow's own commit —github.job_workflow_shaevaluates to empty).Open questions for review:
phpstan-pre-push.sh; it can either adopt the canonical single-file layout or call withverify-hook: false.@main, matching how these workflows already consumescripts/bash/*— happy to pin to tags instead if preferred.phpcs.ymlandmatomo-tests.yml, which have the same duplication shape.Ticket: PG-4897.
Checklist
Review