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

Fix #18: Correct extensions not scanned, allow extensions to be overridden #27

Merged
merged 2 commits into from
Oct 11, 2021

Conversation

danepowell
Copy link
Collaborator

@danepowell danepowell commented Oct 5, 2021

Fixes #18 by removing the list of extensions to scan from our rulesets. Users must now specify the extensions to scan via the --extensions arg or in phpcs.xml.

I think this should be part of a minor release, since the effects of the breaking change are identical to the bug being fixed (i.e. .module files silently failing to be scanned). The release notes just need to call out this change and the need to manually specify extensions going forward.

@wu-edward
Copy link

@danepowell I just tested as a patch to my BLT 13 project, and now module files are sniffed as expected.

I don't have an opinion on whether it's a minor release, as I can see issues either way.

Copy link
Contributor

@TravisCarden TravisCarden left a comment

Choose a reason for hiding this comment

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

Wow. Nice to see this come up again just in time to celebrate the three-year anniversary of the upstream bug report. 😳 I guess they did say it won't be properly fixed until PHPCS 4.0. Oh well.

This seems like a fine solution to me. Your assessment of the impact of the change seems right to me, @danepowell: a minor release with a clear callout in the release notes seems good.

I just have one suggestion: I think we can simplify the configuration for the end user by putting the rule selection and extensions in the same place like this:

<!-- Uncomment your chosen standard and the filename extensions corresponding to it. -->
<!-- @see https://github.com/acquia/coding-standards-php/issues/18 for background on filename extensions. -->
<rule ref="AcquiaDrupalStrict"/><arg name="extensions" value="php,module,inc,install,test,profile,theme,css,info,txt,md,yml"/>
<!-- <rule ref="AcquiaDrupalTransitional"/><arg name="extensions" value="php,module,inc,install,test,profile,theme,css,info,txt,md,yml"/> -->
<!-- <rule ref="AcquiaPHP"/><arg name="extensions" value="php,inc,test,css,txt,md,yml"/> -->

Then they just uncomment the one line they want. What do you think?

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.

AcquiaPHP extensions takes precedence over AcquiaDrupalStrict extensions list
3 participants