Use extension list instead of ignore for phpcs #148
Conversation
| @@ -228,9 +228,8 @@ phpcs: | |||
| # Space-separated list of directories to review. | |||
| directories: "${drupal.root}/modules/custom ${drupal.root}/themes/custom ${drupal.root}/profiles/custom" | |||
|
|
|||
| # Comma-separated list of patterns for files and directories to exclude from the | |||
There was a problem hiding this comment.
Existing build.xml files use this property -- so we should keep it, but add a comment saying it will be deprecated in 3.0 (which... we don't have a roadmap really so who knows when that would be...)
| @@ -132,7 +132,7 @@ | |||
| </phplint> | |||
|
|
|||
| <!-- Run PHP Code Sniffer. --> | |||
| <property name="phpcs.command" value="vendor/bin/phpcs --standard=${phpcs.standard} --ignore=${phpcs.ignore} ${phpcs.directories}" /> | |||
| <property name="phpcs.command" value="vendor/bin/phpcs --standard=${phpcs.standard} --extensions=${phpcs.extensions} ${phpcs.directories}" /> | |||
|
Re: your questions... Including js files in code reviews - I think we should go ahead and add this. Existing projects will need to change their build.xml file to get this change anyway, and we should have this as a default on new projects Removing the |
|
@becw I restored the default value for |
Due to https://www.drupal.org/project/coder/issues/3074176 ("Allow extensions to be overridden in contrib phpcs.xml file") included in Coder 8.37, the default list of extensions checked by
phpcshas changed.This removes the
ignoreconfiguration option and replaces it withextensionswith a default that matches the original list in Coder.Note: This will require re-installing The Build or manually updating
build.xmlon projects.Open questions
Should we add
jsto the list of extensions? The default behavior of Coder 8.3.7 checking JS files generated a lot of errors on one project where it was used, but it could be useful to have in the list.Should we remove
ignorefrom the defaults? This would cause projects that don't update theirbuild.xmlto start checking.mdfiles, so it might be best to leave it in? Depending on the answer to the first question above, we could also add*.jsto the list of ignored extensions to avoid checking files that weren't previously checked.