[CMS-415] Update Installation and add Testing sections#27
Conversation
uberhacker
changed the title
| ## Installation | ||
| For help installing, see [Manage Plugins](https://pantheon.io/docs/terminus/plugins/) | ||
|
|
||
| ### Installing Secrets 3.x: |
There was a problem hiding this comment.
Only the Build Tools plugin has different major versions that align to different Terminus versions. Other plugins have a single version that may be installed on Terminus 3 or Terminus 2.
There was a problem hiding this comment.
Would Installing Secrets in Terminus 3.x be better?
There was a problem hiding this comment.
Yeah; see README of the Terminus plugin example, and follow that format.
|
|
||
| ```bash | ||
| mkdir -p ~/.terminus/plugins | ||
| composer create-project -d ~/.terminus/plugins pantheon-systems/terminus-secrets-plugin:~1 |
There was a problem hiding this comment.
I don't think we need the ~1, do we?
Update README.md as suggested by Greg
uberhacker
force-pushed
the
feature/CMS-415-Update-terminus-secrets-plugin
branch
from
0d76aa6 to
50c53ac
Compare
| "cs": "PATH=tools/bin:$PATH phpcs --standard=PSR2 -n src Commands", | ||
| "cbf": "PATH=tools/bin:$PATH phpcbf --standard=PSR2 -n src Commands", | ||
| "functional": "PATH=tools/bin:$PATH bats tests/functional", | ||
| "cs": "PATH=vendor/bin:$PATH phpcs --standard=PSR2 -n src Commands", |
There was a problem hiding this comment.
If you want to move the tools to vendor/bin, then you will also need to add them to require-dev. However, this won't work for bats. If we want to remove the "install tools" step, maybe we should switch to phpunit-based testing.
There was a problem hiding this comment.
This is actually correcting a bug. The phpcs and phpcbf executables are located in vendor/bin, not tools/bin. It might make sense to remove the PATH=... line and just call vendor/bin/phpcs directly.
There was a problem hiding this comment.
require-dev is empty in this composer.json, ergo, phpcs et. al. probably won't be found there.
The design / setup of the tests were intended to allow the tests to be run without installing any dev dependencies into the vendor directory. This was done due to the limitations of Terminus 2, where dev dependencies would often conflict with Terminus' own dependencies. The way the plugin avoided dev dependencies was via the install-bats commands & etc., which installs them to tools. Setting the PATH and then calling bats allowed the tests to be run in environments that had a global bats and an empty tool directory, or in environments that used the install-bats script to install it to tools.
Since Terminus 3 no longer has the restriction on dev dependencies, we could do as you suggest and just call vendor/bin/phpcs directly. However, to do that you'd have to add phpcs to require-dev. Also, bats can't be installed via Composer. I suppose that you could keep the install-bats trick and use require-dev for just the PHP tools if you wanted.
It would be a good idea to turn on testing for GitHub actions, so that we don't need to worry about the Circle permissions. It wouldn't be hard to make Circle run again, but GitHub actions are probably a better example for open source repositories.
There was a problem hiding this comment.
Thank you for the feedback. I'm a bit confused. Doesn't COMPOSER_BIN_DIR=../../vendor/bin cause phpcs to be installed in the vendor directory? This is what I experienced in my testing. If you execute composer install-tools then vendor/bin/phpcs works, hence the proposed change to fix. What are you thoughts regarding the new Testing section in README.md?
There was a problem hiding this comment.
@greg-1-anderson: How should we proceed with this PR? The most recent merge has created a merge conflict. Should I just close?
2 participants
Modified tests to work with Terminus 3.