Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use existing tools/config for PHPCS #77

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

vauxia
Copy link
Collaborator

@vauxia vauxia commented Feb 8, 2025

This PR amends some of the recently-added pre-commit hook behavior:

  • Remove explicit composer requirement to drupal/coder: It is already a dependency of drupal/core-dev, which is required in the dev section.
    • It can get messy to require something that's already a prerequesite
    • It's not great to put drupal/coder on a production environment, which is why drupal/core-dev is in dev.
  • Look for a phpcs.xml file and respect the settings therein. This was already set up here: PHPCS configuration #15
    • Ensures that phpcs works consistently when using ddev phpcs, pre-commit hooks, PHPStorm's auto-validation, etc.
    • Example: runs phpcs with both Drupal and DrupalPractice profiles, as configured in the settings file
  • Look for a .ddev folder and a working ddev executable, and run ddev exec phpcs when available.
    • Not all developers have PHP binaries, or use similar versions, outside of the ddev containers.
    • Ensures a consistent version of PHP for testing + deployment
  • Also fixes PHP Fatal error: Uncaught Error: Class "\PHP_CodeSniffer\Filters\gitstaged" not found in /home/allie/dev/civicactions/pgov-cms/vendor/squizlabs/php_codesniffer/src/Files/FileList.php:90 error

These changes were added conditionally, which means that neither phpcs.xml nor ddev are required. If these are not present, respect the original behavior ( 'Drupal' standard, vendor/bin/phpcs executable )

@vauxia
Copy link
Collaborator Author

vauxia commented Feb 11, 2025

@iris-i I'm hoping you can take a look at this if you have time. At present, I'm not able to commit anything without adding a --no-verify because of case sensitivity on my linux machine.

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.

1 participant