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

Feature Idea: PAReview.sh as installed extra #1197

Closed
PieterDC opened this Issue Mar 3, 2017 · 10 comments

Comments

4 participants
@PieterDC

PieterDC commented Mar 3, 2017

Issue Type

Feature Idea

Summary

Support PAReview.sh as optional extra because the Drupal project application checklist recommends it as a way to check coding standards and style. And it's better to be able to check those things before pushing Drupal code to a repo.

@geerlingguy

This comment has been minimized.

Owner

geerlingguy commented Mar 3, 2017

@PieterDC - Definitely a valid request. I've considered adding a few workflow-specific tools to Drupal VM in the past (it is, after all, "Drupal" VM and not "generic PHP CMS VM"). However, since the tool would probably be used only by a small percentage of all Drupal VM users, I'd rather not maintain a separate role and/or functionality to grab the script, put it in place, etc.

However, I would like to document how it can be done quickly/easily in this ticket for those who want to use it—either via a post provision script or task file...

@geerlingguy geerlingguy added the question label Mar 3, 2017

@geerlingguy

This comment has been minimized.

Owner

geerlingguy commented Mar 3, 2017

Also, note that this script is a wrapper around some tooling that is usually best as part of a Drupal project itself (e.g. how Acquia BLT wraps up coding standards reviews, PHPUnit tests, and Behat tests in a Drupal project codebase)—therefore I've been shying away from adding things like that directly to Drupal VM.

@geerlingguy

This comment has been minimized.

Owner

geerlingguy commented Mar 13, 2017

Working on this now... I think we can get a script in the example scripts dir that will do 99% of the work, and then you just copy and paste a few variables into your config.yml to get the rest done :)

Testing this on Honeypot now. Apparently I still have a master branch lurking in that project! See: https://www.drupal.org/node/2860293

@geerlingguy

This comment has been minimized.

Owner

geerlingguy commented Mar 13, 2017

Tested and working! Basically, you add the following to config.yml, and you're off to the races!

post_provision_scripts:
  - "../examples/scripts/pareview.sh"

composer_global_packages:
  - { name: hirak/prestissimo, release: '^0.3' }
  - { name: drupal/coder, release: '^8.2' }

nodejs_version: "6.x"
nodejs_npm_global_packages:
  - name: eslint

See full docs here: https://github.com/geerlingguy/drupal-vm/blob/master/examples/scripts/pareview.sh

@PieterDC

This comment has been minimized.

PieterDC commented Mar 14, 2017

Wow. Thanks!

I created an internal task for our team to check this out with a suggestion on how to integrate this in our continuous deployment flow.

@geerlingguy

This comment has been minimized.

Owner

geerlingguy commented Mar 14, 2017

Blog post with a detailed guide and more notes: Drupal's Contrib floodgates are open, PAReview your projects in Drupal VM!.

@attheshow

This comment has been minimized.

attheshow commented Apr 17, 2017

A note to anyone else trying to use this...

The mention of eslint in the config.yml needs to be formatted like this:

nodejs_npm_global_packages:
  - name: eslint

See: #1301

@geerlingguy

This comment has been minimized.

Owner

geerlingguy commented Apr 17, 2017

@attheshow - Is this documented incorrectly somewhere? I think I fixed my blog post (the one place I know I had mistyped the variable), but I want to try to make it so others don't run into the same issue :)

@oxyc

This comment has been minimized.

Collaborator

oxyc commented Apr 17, 2017

In your comment here #1197 (comment)

@geerlingguy

This comment has been minimized.

Owner

geerlingguy commented Apr 18, 2017

@oxyc oops! Fixed it.

sonfd added a commit to sonfd/drupal-vm that referenced this issue May 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment