Adds PHPStan to Slack plugin by AltamashShaikh · Pull Request #2 · matomo-org/plugin-Slack

AltamashShaikh · 2025-08-26T02:43:53Z

Description:

Adds PHPStan to Slack plugin

Review

james-hill-matomo

Looks good. Given this is a brand new plugin, could we have a higher level as base?

Can bump the level that high with (almost) no errors.

james-hill-matomo · 2025-08-26T22:40:51Z

@AltamashShaikh Can you add the git hooks and check that they work with your non ddev setup? They should run successfully on git pushing so long as you've modified or created a php file.

git config core.hooksPath .git-hooks-matomo

I've bumped the base level up to 5 as I was able to do so with minimal code fixes (see commit).

james-hill-matomo

@AltamashShaikh Happy to merge if you're OK with my changes, and can you confirm that the pre-push git hook works for you?

AltamashShaikh · 2025-08-27T04:07:31Z

@AltamashShaikh Can you add the git hooks and check that they work with your non ddev setup? They should run successfully on git pushing so long as you've modified or created a php file.
git config core.hooksPath .git-hooks-matomo
I've bumped the base level up to 5 as I was able to do so with minimal code fixes (see commit).

@james-hill-matomo I gave execute permission to pre-push file chmod +x .git-hooks-matomo/pre-push and updated the pre-push file to make git push work.

…r both created and updated

AltamashShaikh · 2025-08-27T06:32:11Z

@james-hill-matomo I updated the pre-push file, earlier it was running only for 5.x-dev branch corrected to pick the current branch and changes around plugin_path, please check on DDEV for me its working fine.

AltamashShaikh · 2025-08-27T07:07:28Z

+echo "Running pre-commit hook in repo: $REPO_DIR"
+
+if [[ "$REPO_DIR" =~ /plugins/(.*) ]]; then
+    PLUGIN_PATH=${REPO_DIR}


Changed to get plugin_path

AltamashShaikh · 2025-08-27T07:07:59Z

+PHPSTAN_CREATED_CONFIG=phpstan/phpstan.created.neon
+BRANCH_NAME=$(git branch --show-current)
+if [[ -f "$PHPSTAN_CREATED_CONFIG" ]]; then
+  CHANGED_FILES=$(git diff --name-only ${BRANCH_NAME} --diff-filter=A | grep '\.php$' || true)


Added branch name. earlier it was referring to 5.x-dev

AltamashShaikh · 2025-08-27T07:08:21Z

+  if [ -z "$CHANGED_FILES" ]; then
+    echo "No created PHP files"
+  else
+    echo "Running PHPstan at a very high level on new files"


Removed filtering of changed files

That adds the plugin path, and is needed for it to work in ddev. I hope I fixed it for non ddev by putting a "cd" at the start of the script.

I removed it because of cd

AltamashShaikh · 2025-08-27T07:08:33Z

+### Run PHPStan on modified files. ###
+PHPSTAN_MODIFIED_CONFIG=phpstan/phpstan.modified.neon
+if [[ -f "$PHPSTAN_MODIFIED_CONFIG" ]]; then
+  CHANGED_FILES=$(git diff --name-only ${BRANCH_NAME} --diff-filter=CM | grep '\.php$' || true)


Added branch name here

AltamashShaikh · 2025-08-27T07:08:46Z

+  if [ -z "$CHANGED_FILES" ]; then
+    echo "No changed PHP files"
+  else
+    echo "Running PHPstan on modified files"


Removed filtering of files

That adds the plugin path, and is needed for it to work in ddev. I hope I fixed it for non ddev by putting a "cd" at the start of the script.

AltamashShaikh · 2025-08-27T07:39:36Z

+        # ../../ does not actually seem to give us anything
+        # that ../plugins/ does not, but including it for
+        # completeness.  It does not seem to slow down performance.
+        - ../../


@james-hill-matomo Is this needed ?

We should be getting everything we need from ../../bootstrap-phpstan.php (for core) and ../plugins/ for non core plugins we include.

Including ../../ is maybe not necessary, but it doesn't seem to slow things down either.

@james-hill-matomo Was getting recursive error after recent change
I changed to below

Suggested change

- ../../

- .

james-hill-matomo · 2025-08-29T04:25:21Z

@AltamashShaikh Ddev broke for me, so I tweaked the files again - can you test on your local and let me know? We can do screenshare if that's easier.

james-hill-matomo · 2025-08-29T04:26:34Z

@james-hill-matomo I updated the pre-push file, earlier it was running only for 5.x-dev branch corrected to pick the current branch and changes around plugin_path, please check on DDEV for me its working fine.

That branch change didn't work for me - the idea is that the system always compares your current branch with 5.x-dev .

I've taken your change and made it so that it figures out the default branch, to save us some time when 6.x comes out.

AltamashShaikh · 2025-08-29T05:14:28Z

+### Run PHPStan on newly created files. ###
+
+PHPSTAN_CREATED_CONFIG=phpstan/phpstan.created.neon
+MAIN_BRANCH=$(git symbolic-ref --short refs/remotes/origin/HEAD)


@james-hill-matomo getting this error when running MAIN_BRANCH=$(git symbolic-ref --short refs/remotes/origin/HEAD)
fatal: ref refs/remotes/origin/HEAD is not a symbolic ref

we can hardcode it, we need to change only once in future for 6.x

AltamashShaikh · 2025-08-29T05:36:30Z

+if command -v php >/dev/null 2>&1; then
+    cd "${MATOMO_DIR}"
+    if [ -f "vendor/bin/phpstan" ]; then
+      COMMAND="vendor/bin/phpstan"


@james-hill-matomo This should have full path

Suggested change

COMMAND="vendor/bin/phpstan"

COMMAND="${MATOMO_DIR}/vendor/bin/phpstan"

AltamashShaikh

@james-hill-matomo I made few tweaks to work it for non-ddev, can you check again with ddev ?

james-hill-matomo · 2025-08-31T21:20:49Z

@AltamashShaikh Works for me too !

Adds PHPStan to Slack plugin

2288928

AltamashShaikh mentioned this pull request Aug 26, 2025

Initial PR to send scheduled reports to a Slack channel, #PG-4491 #1

Merged

AltamashShaikh added the Needs Review For pull requests that need a code review. label Aug 26, 2025

AltamashShaikh requested a review from a team August 26, 2025 02:45

james-hill-matomo approved these changes Aug 26, 2025

View reviewed changes

Comment thread phpstan/phpstan.created.neon Outdated

Changed the level to 8

1811a57

AltamashShaikh requested a review from james-hill-matomo August 26, 2025 03:07

Bumped phpstan level up to 5

079d54e

Can bump the level that high with (almost) no errors.

james-hill-matomo force-pushed the PG-4491-phpstan branch 11 times, most recently from 2a2cda8 to 1ddee14 Compare August 26, 2025 22:35

Don't check github action with phpstan

eff2eee

james-hill-matomo force-pushed the PG-4491-phpstan branch from 1ddee14 to eff2eee Compare August 26, 2025 22:37

james-hill-matomo requested a review from snake14 August 26, 2025 22:41

james-hill-matomo approved these changes Aug 26, 2025

View reviewed changes

AltamashShaikh added 4 commits August 27, 2025 07:46

Test changes for multiple channels

8696772

Reverted changes

60c76ae

Adds the multiple channel logic back

ca92325

Fixes for pre-push hook to work locally

700046d

AltamashShaikh requested a review from james-hill-matomo August 27, 2025 04:07

Changes to work on all branches and reduced the phpstan level to 5 fo…

ad666c0

…r both created and updated

AltamashShaikh added 3 commits August 27, 2025 12:04

Force to 8 to ensure github action is working

72b9ff1

test for GH action

aa194b2

More fixes around pre-push and reverted to level 5

a24df9c

AltamashShaikh commented Aug 27, 2025

View reviewed changes

Comment thread .git-hooks-matomo/pre-push

AltamashShaikh commented Aug 27, 2025

View reviewed changes

AltamashShaikh and others added 3 commits August 27, 2025 13:10

Changed level to 5 for update

4be146a

Get correct branch for comparison

83122ca

PHPStan work correctly with DDEV

592fb1f

james-hill-matomo force-pushed the PG-4491-phpstan branch from eb5ed2e to 592fb1f Compare August 29, 2025 04:19

Fixed context of cd for non ddev phpstan

4d1a6db

AltamashShaikh commented Aug 29, 2025

View reviewed changes

Changes for pre-push to work for non-dev

3194ae4

AltamashShaikh commented Aug 29, 2025

View reviewed changes

james-hill-matomo merged commit ebbbc48 into PG-4491-schedule-report-slack Aug 31, 2025
8 checks passed

james-hill-matomo deleted the PG-4491-phpstan branch August 31, 2025 21:21

	COMMAND="vendor/bin/phpstan"
	COMMAND="${MATOMO_DIR}/vendor/bin/phpstan"

Conversation

AltamashShaikh commented Aug 26, 2025

Description:

Review

Uh oh!

james-hill-matomo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

james-hill-matomo commented Aug 26, 2025

Uh oh!

james-hill-matomo left a comment

Choose a reason for hiding this comment

Uh oh!

AltamashShaikh commented Aug 27, 2025

Uh oh!

AltamashShaikh commented Aug 27, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

james-hill-matomo commented Aug 29, 2025

Uh oh!

james-hill-matomo commented Aug 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AltamashShaikh left a comment

Choose a reason for hiding this comment

Uh oh!

james-hill-matomo commented Aug 31, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants