search

PHPCSStandards / composer-installer

Composer installer for PHP_CodeSniffer coding standards
https://packagist.org/packages/dealerdirect/phpcodesniffer-composer-installer
MIT License
542 stars 36 forks source link

Allow use within PHP_CodeSniffer itself #215

Open fredden opened 7 months ago

fredden commented 7 months ago

Proposed Changes

While looking into changing the coding standard used for PHP_CodeSniffer itself, I noticed that this plug-in doesn't currently work in that scenario. This pull request allows this plug-in to be used within the PHP_CodeSniffer repository.

Related Issues

Related to https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/122#pullrequestreview-1762881173

jrfnl commented 7 months ago

@fredden I'm not sure I see the point of this change ?

  1. PHPCS itself does not (currently) use outside standards for its own CS check, so making this change at this time is not needed.
  2. Making a change in this package for a potential future situation which is specific to only one potential user, while there are other options available to that user which will work just as well... doesn't seem to justify the maintenance burden of the change.

What am I missing ?

fredden commented 7 months ago
  1. PHPCS itself does not (currently) use outside standards for its own CS check, so making this change at this time is not needed.

Correct. I was working on changing that, as you'd suggested that in the future it would be good to use the PHPCSDevCS standard internally for PHP_CodeSniffer. That change is low priority, and involves a large change to the code-base, so is unlikely to happen in the short term. In order to avoid duplication, I installed PHPCSDevCS via Composer, and found this incompatibility.

  1. Making a change in this package for a potential future situation which is specific to only one potential user, while there are other options available to that user which will work just as well... doesn't seem to justify the maintenance burden of the change.

Making this change seemed cleaner and more robust than setting a hard-coded installed_paths in several places within PHP_CodeSniffer. Depending on how that would be handled there, anyone wanting to contribute to that project may also need to perform an additional manual set-up step. Note too that the PHPCSDevCS standard depends on at least one other standard via Composer which will need to be included in the installed_paths setting.

jrfnl commented 7 months ago
  1. PHPCS itself does not (currently) use outside standards for its own CS check, so making this change at this time is not needed.

Correct. I was working on changing that, as you'd suggested that in the future it would be good to use the PHPCSDevCS standard internally for PHP_CodeSniffer. That change is low priority, and involves a large change to the code-base, so is unlikely to happen in the short term. In order to avoid duplication, I installed PHPCSDevCS via Composer, and found this incompatibility.

  1. Making a change in this package for a potential future situation which is specific to only one potential user, while there are other options available to that user which will work just as well... doesn't seem to justify the maintenance burden of the change.

Making this change seemed cleaner and more robust than setting a hard-coded installed_paths in several places within PHP_CodeSniffer. Depending on how that would be handled there, anyone wanting to contribute to that project may also need to perform an additional manual set-up step. Note too that the PHPCSDevCS standard depends on at least one other standard via Composer which will need to be included in the installed_paths setting.

I understand what you are trying to do, but this is not the time. Let's focus on what's in front of us right now first.

It may still be years before that CS change in PHPCS itself happens, it may never happen. If anything, in my mind, it will likely be an iterative process with small changes to normalize the code a bit more over time, bit by bit, until it is at a point that a switch would be realistic.