Skip to content

Test adding PHPCS check to the pre-push hook#18

Merged
snake14 merged 5 commits into
5.x-devfrom
try-PHPCS-git-hook
Nov 3, 2025
Merged

Test adding PHPCS check to the pre-push hook#18
snake14 merged 5 commits into
5.x-devfrom
try-PHPCS-git-hook

Conversation

@snake14
Copy link
Copy Markdown
Contributor

@snake14 snake14 commented Oct 29, 2025

Description

Testing out adding the PHPCS check to the pre-push check. To test a failure, I simply made a copy of the pre-push file, named it pre-commit, and moved a curly brace in another file to the same line as the method signature.

@snake14 snake14 added the Needs Review For pull requests that need a code review. label Oct 29, 2025
@snake14
Copy link
Copy Markdown
Contributor Author

snake14 commented Oct 30, 2025

@james-hill-matomo @AltamashShaikh I mentioned that I used the pre-commit hook to test this change and now I'm thinking that maybe I should simply commit that and leave pre-push as it was. I could even change it to run PHPCBF to automatically fix any issues before commit. What do you think?

@AltamashShaikh
Copy link
Copy Markdown
Contributor

@james-hill-matomo @AltamashShaikh I mentioned that I used the pre-commit hook to test this change and now I'm thinking that maybe I should simply commit that and leave pre-push as it was. I could even change it to run PHPCBF to automatically fix any issues before commit. What do you think?

Makes sense 👍

@snake14
Copy link
Copy Markdown
Contributor Author

snake14 commented Oct 30, 2025

@AltamashShaikh @james-hill-matomo Please check the latest changes and let me know if everything looks good.

@james-hill-matomo
Copy link
Copy Markdown

pre-commit hook is great if it runs quickly.

Copy link
Copy Markdown

@james-hill-matomo james-hill-matomo left a comment

Choose a reason for hiding this comment

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

@snake14 this doesn't run on my PC as I don't have PHP locally, just via ddev.

I can work on this later to work on my setup.

@snake14
Copy link
Copy Markdown
Contributor Author

snake14 commented Oct 31, 2025

@snake14 this doesn't run on my PC as I don't have PHP locally, just via ddev.

I can work on this later to work on my setup.

@james-hill-matomo I wonder if it's as simple and using ddev exec if DDEV is running. I can try making that change in a little while.

@james-hill-matomo
Copy link
Copy Markdown

@snake14 this doesn't run on my PC as I don't have PHP locally, just via ddev.
I can work on this later to work on my setup.

@james-hill-matomo I wonder if it's as simple and using ddev exec if DDEV is running. I can try making that change in a little while.

@snake14 Also have to change the paths. This manual test worked for me:

ddev exec phpcbf plugins/OpenApiDocs --standard=plugins/OpenApiDocs/phpcs.xml

You can see the pre-push checks for PHP, although they're pretty straight forwards.

Copy link
Copy Markdown
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

Works fine for me, just need to check with ddev

image

@snake14
Copy link
Copy Markdown
Contributor Author

snake14 commented Oct 31, 2025

@AltamashShaikh @james-hill-matomo I'm pretty sure that I've got it working for both local PHP and DDEV now. I also updated to stop if PHPCBF finds anything to correct. I thought about auto-adding, but figured that it would be better to have the dev review and add manually.

Copy link
Copy Markdown
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

Still works fine for me 👍

@snake14
Copy link
Copy Markdown
Contributor Author

snake14 commented Nov 3, 2025

@james-hill-matomo Please review/test again and let me know if there are any issues.

Copy link
Copy Markdown

@james-hill-matomo james-hill-matomo left a comment

Choose a reason for hiding this comment

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

Works for me now, thank you!

@snake14 snake14 merged commit e143f42 into 5.x-dev Nov 3, 2025
7 checks passed
@snake14 snake14 deleted the try-PHPCS-git-hook branch November 3, 2025 20:25
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