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

Fixes #3217 to provide acsf-init-verify as part of git hooks and blt … #3218

Merged
merged 1 commit into from
Jan 4, 2019

Conversation

mikemadison13
Copy link
Contributor

@mikemadison13 mikemadison13 commented Nov 6, 2018

…validate.

Fixes #3217

Changes proposed:

(What are you proposing we change?)

  • adds validate / acsf to template blt.yml
  • adds a acsf-init-validate command to blt validate
  • adds the acsf validation command to validate all WHEN acsf validation is enabled in blt.yml

Steps to verify the solution:

(How does someone verify that this does what you think does?)

  1. update your blt.yml file to include:
validate:
  acsf: true
  1. run either blt validate if 1 was done, you should see the output like:
Validating ACSF settings...
[Acquia\Blt\Robo\Tasks\DrushTask] Running /var/www/<project>/vendor/bin/drush @self --include=modules/contrib/acsf/acsf_init acsf-init-verify --no-interaction --ansi in /var/www/<project>/docroot
 [success] acsf-init required files ok

3. manually run blt tests:acsf:validate

Warning, should not be run on non-acsf projects (which is why it defaults to not running). 

 

@mikemadison13
Copy link
Contributor Author

Outstanding question if it makes sense to do this as part of the pre-commit hook or not. It seems to me that just having the safety check in blt validate (so builds fail) is sufficient.

@mikemadison13
Copy link
Contributor Author

Would love feedback by common ACSF users (@msherron, @danepowell, @lcatlett, @arknoll, etc.)

@danepowell
Copy link
Contributor

I like the idea of this, primarily to solve the problem of accidentally updating the ACSF module and failing to re-run acsf-init.

I don't think a pre-commit hook is the logical place for this. pre-commit hooks should be reserved for checks for the most frequent problems like syntax errors and bad commit messages. Failing to run acsf-init is something that happens once or twice per project. Better to do that in travis and avoid slowing down commits any further.

*/
public function validateAcsf() {
// To enable this command, set validate.acsf to TRUE in blt.yml.
$acsf_config = $this->getConfigValue('validate.acsf');
Copy link
Contributor

@grasmash grasmash Dec 13, 2018

Choose a reason for hiding this comment

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

This seems like it's in the wrong place. The check should be in AllCommand.php. That way, people can still voluntarily tests:acsf:validate without needing to modify yaml.

@mikemadison13 mikemadison13 merged commit 5ce159b into acquia:10.0.x Jan 4, 2019
mikemadison13 added a commit to mikemadison13/blt that referenced this pull request Feb 21, 2019
mikemadison13 added a commit to mikemadison13/blt that referenced this pull request Feb 21, 2019
mikemadison13 added a commit that referenced this pull request Feb 25, 2019
* Fixes #3265 to expand alias documentation. (#3399)

* Fixes #3401 for 9.2.x to update devel.

* Moved pipelines-deploy from build event to post-deploy event (#3374)

* Docksal documentation (#3268)

* Add links to docsal and ddev pages.

* Updates ignore-existing to include .editorconfig. (#3300)

Fixes #3169 
--------

Changes proposed:
---------
(What are you proposing we change?)

- adds the .editorconfig file to the ignore list for blt updates

* Updating BLT's docs to remove nvm and point to Cog's documentation for theme installation. (#3313)

* Fix broken link to Cloud Hooks docs. (#3331)

* Added steps to install nfs-utils and enable access via firewalld; without it DrupalVm gets stuck at mounting NFS (#3376)

* Updates release and readme docs. (#3382)

* Updates Travis and Pipelines to PHP 7.1 by default.

* Adds FAQ for drush 8/9 permission denied issue. (#3264)

* Add setup instructions for PHPStorm + PHPUnit (#1787) (#3198)

* Update INSTALL.md (#3273)

* Speed up deploys with shallow fetching (#3271)

Before:
```
real	4m11.340s
user	1m3.430s
sys	1m23.638s
```

After:
```
real	3m14.776s
user	0m47.570s
sys	1m6.415s
```

* Fixes #3217 to provide acsf-init-verify as part of git hooks and blt validate. (#3218)

* Updating Drupal VM config versions. (#3328)

* Fixes #3345 to run db updates even when config strategy is none. (#3363)

* Fixes #3362 to better identify why the site URI is not found. (#3369)

* Test tests:acsf:validate (#3392)

* Test tests:acsf:validate

* Delete README.md

Remove cloud hook readme.

* Update AcsfHooksTest.php

* Update AcsfHooksTest.php

* Update DeployTest.php

* Fixes #3383 to backport Pipelines update to 9.2.x.

* Increase wait timeout. (#3163)

* Fixes #3122 to remove misleading jmeter example. (#3230)

* Fixes #3223 to update git docs. (#3226)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A feature or feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants