Skip to content

Sync PHPStan pre-push hook and workflow fixes [no_release] - #58

Merged
snake14 merged 12 commits into
6.x-devfrom
PG-4897-hook-fixes
Sep 1, 2026
Merged

Sync PHPStan pre-push hook and workflow fixes [no_release]#58
snake14 merged 12 commits into
6.x-devfrom
PG-4897-hook-fixes

Conversation

@snake14

@snake14 snake14 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Description

Applies the PHPStan tooling fixes from the PG-4897 rollout template to this plugin's copy:

  • The pre-push hook now diffs against the merge base with origin/5.x-dev instead of the local 5.x-dev branch, which could be stale or missing and silently widened the analysis to files the push doesn't touch. It now fails with a clear message when the merge base can't be resolved.
  • The hook falls back to ddev when a local PHP exists but PHPStan isn't installed in the Matomo vendor dir (previously this combination dead-ended and the hook reported no way to run PHPStan).
  • The github-action-tests checkout in the PHPStan workflow no longer persists credentials.
  • Newly created files are now checked at PHPStan level 9: new code carries no pre-existing debt, so it is held to the strictest level.

Tooling-only change: no version bump or changelog entry (no_release).

Issue No

Related to PG-4897 (JIRA)

Steps to Replicate the Issue

  1. Push a branch from a checkout whose local 5.x-dev is stale or missing.
  2. Expected: the pre-push hook analyses only the files the push touches.
  3. Actual: the hook diffed against the stale local branch and analysed unrelated files, or failed to resolve the branch entirely.

Checklist

  • [✔] Tested locally or on demo2/demo3?
  • [NA] New test case added/updated?
  • [NA] Are all newly added texts included via translation?
  • [NA] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
  • [NA] Version bumped?
  • [✔] I have understood, reviewed, and tested all AI outputs before use
  • [✔] All AI instructions respect security, IP, and privacy rules
  • [NA] Documentation updated?

snake14 added 2 commits August 4, 2026 10:08
Diff against the merge base with origin/5.x-dev instead of the possibly stale
local branch, fall back to ddev when local PHP lacks PHPStan, and stop
persisting credentials on the github-action-tests checkout.
New files carry no pre-existing debt, so hold them to the strictest level.
@snake14 snake14 added the Needs Review For pull requests that need a code review. label Aug 4, 2026
@snake14
snake14 requested a review from a team August 4, 2026 01:06
snake14 added 3 commits August 4, 2026 13:52
Uncommitted local changes were analyzed and could block a push for
files the push doesn't contain. Also corrects the hook's log label.
Pushing another local branch, or several refs at once, analysed the
checked-out branch instead of what was actually pushed.
A plugin cloned outside a Matomo checkout cannot be checked, which is
no reason to block its pushes.
@lachiebol

Copy link
Copy Markdown
Contributor

@snake14 Should we update these PRs once matomo-org/github-action-tests#44 is merged?

@snake14

snake14 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@snake14 Should we update these PRs once matomo-org/github-action-tests#44 is merged?

I had the same thought and Claude recommended merging these and then doing another round after the action-tests changes are merged. What do you think @AltamashShaikh ?

@AltamashShaikh

Copy link
Copy Markdown
Contributor

@snake14 I think we should merge matomo-org/github-action-tests#44 first and update plugins later. Saves review time.

@snake14 snake14 removed the Needs Review For pull requests that need a code review. label Aug 19, 2026
@snake14
snake14 removed the request for review from a team August 19, 2026 04:06
@snake14
snake14 marked this pull request as draft August 19, 2026 04:06
The vendored copy predated the MAIN_BRANCH derivation, so on a 6.x-dev plugin it
diffed against 5.x-dev, and it also lacked the ddev-running, NUL-safe-path and
renamed-file fixes.
@snake14
snake14 changed the base branch from 5.x-dev to 6.x-dev August 30, 2026 21:43
The focus ring left by picking the option was painted in some runs and not
others, so the screenshot alternated between two states.
Two consecutive runs produce this image byte-identically, so it is the settled
state, not a race; the previous expectation came from a single earlier run. The
blur added to chase that is removed -- it changed nothing, so the outline is not
painted by focus on the active element.
@snake14
snake14 marked this pull request as ready for review August 31, 2026 03:44
@snake14 snake14 added the Needs Review For pull requests that need a code review. label Aug 31, 2026
@snake14
snake14 requested a review from a team August 31, 2026 03:52
AltamashShaikh
AltamashShaikh previously approved these changes Sep 1, 2026
@snake14
snake14 merged commit 502b319 into 6.x-dev Sep 1, 2026
13 checks passed
@snake14
snake14 deleted the PG-4897-hook-fixes branch September 1, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review For pull requests that need a code review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants