acquia / blt

Acquia's toolset for automating Drupal 10 development, testing, and deployment.
https://docs.acquia.com/blt/
GNU General Public License v2.0
443 stars 395 forks source link

Disable haltonwarning : blt validate:phpcs #1070

Closed nirojghale-iheartmedia closed 7 years ago

nirojghale-iheartmedia commented 7 years ago

My system information:

Output of blt doctor:

+--------------------------+---------------------------------------------------------------------------------+
| Check                    | Outcome                                                                         |
+--------------------------+---------------------------------------------------------------------------------+
| checkDocrootExists       | Found docroot.                                                                  |
| checkCoreExists          | Drupal core exists                                                              |
| checkSettingsFile        | BLT settings are included in settings file.                                     |
| checkLocalSettingsFile   | Found your local settings file.                                                 |
| checkLocalDrushFile      | Found your local drush settings file.                                           |
| checkUri                 | $options['uri'] is set.                                                         |
| checkUriResponse         | Received a response from site http://local.iheartmedia.com.                     |
| checkFileSystem:%files   | Public files directory is writable.                                             |
| checkFileSystem:%private | Private files directory is writable.                                            |
| checkFileSystem:%temp    | Temporary files directory is writable.                                          |
| checkDbConnection        | Connected to database.                                                          |
| checkDrupalBootstrapped  | Bootstrapped Drupal via drush.                                                  |
| checkDrupalInstalled     | Drupal is installed.                                                            |
| checkCaching:page        | Drupal cache is disabled.                                                       |
| checkCaching:css         | CSS preprocessing is disabled.                                                  |
| checkCaching:js          | JS preprocessing is disabled.                                                   |
| checkNvmExists           | NVM does not exist.                                                             |
|                          |                                                                                 |
|                          | It is recommended that you use NVM to manage multiple versions of NodeJS on one |
|                          | machine.                                                                        |
|                          | Instructions for installing NVM can be found at:                                |
|                          |   https://github.com/creationix/nvm#installation                                |
| checkCiConfig            | Git remotes are set in project.yml.                                             |
| checkComposer:require    | acquia/blt is in composer.json's require object.                                |
| checkComposer:plugins    | hirak/prestissimo plugin for composer is installed.                             |
| checkBehat:exists        | Behat local settings file exists.                                               |
| checkProjectYml:keys     | project.yml has no deprecated keys.                                             |
| checkPhpDateTimezone     | PHP setting for date.timezone is correctly set                                  |
+--------------------------+---------------------------------------------------------------------------------+

When I run this command:

blt validate:phpcs

I get the following output:

----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | WARNING | File appears to be minified and cannot be processed
----------------------------------------------------------------------

Time: 24778035 mins, 11.78 secs; Memory: 46Mb

./vendor/acquia/blt/phing/tasks/validate.xml:90:27: phpcodesniffer detected 6 warnings

./vendor/acquia/blt/phing/tasks/validate.xml:79:92: Execution of the target buildfile failed. Aborting.

And I expected this to happen: I want to ignore warning and prevent failing the validation. I just want to validate the actual errors. I changed this haltonwarning="false" in my local and validation passed but I can't do this while runing BLT through Travis. Is there a way to patch the validate.xml file under vender directory or pass any other flag to ignore these warning.

It doesn't make sense for me break the build validation process if there is no actual error.

grasmash commented 7 years ago

There are three ways that you can customize the validate:phpcs target:

  1. You can disable it
  2. You can replace it with your own (override)
  3. You can modify the fileset that the target operates against. If you choose to do this, you'd likely want to ignore all *min.js files.

If you choose option 2, you'd override validate:phpcs:fileset and change the value of haltonwarning.

  <target name="validate:phpcs:fileset" description="Runs PHP Code Sniffer against only custom modules." hidden="true">
    <echo>Code sniffing ${fileset_refid}</echo>
    <!-- Sniff tests. -->
    <phpcodesniffer
      standard="${phpcs.ruleset}"
      showSniffs="false"
      showWarnings="true"
      haltonerror="true"
      haltonwarning="false">
      <fileset refid="${fileset_refid}"/>
      <formatter type="full" usefile="false"/>
    </phpcodesniffer>
  </target>

We could make some of the PHPCS settings into variables that you can set in project.yml, if you're interested.

grasmash commented 7 years ago

We could make some of the PHPCS settings into variables that you can set in project.yml, if you're interested.

Created a PR to introduce these variables.

nirojghale-iheartmedia commented 7 years ago

@grasmash Yes please that would be helpful.

grasmash commented 7 years ago

It was merged into 8.x. Will be in the next release.

nirojghale-iheartmedia commented 7 years ago

@grasmash Can you please let me know your next release date ?

nirojghale-iheartmedia commented 7 years ago

@grasmash Is there a way to ignore *.min.js file in the current version using custom.xml or project.yml file ?