[TASK] Move the PHP code sniffing to GitHub actions #832
Conversation
| uses: actions/cache@v1 | ||
| with: | ||
| path: ~/.composer/cache | ||
| key: php${{ matrix.php-version }}-composer-${{ hashFiles('**/composer.json') }} |
There was a problem hiding this comment.
matrix.php-version seems to be resolving to an empty string, from the logs, e.g. https://github.com/MyIntervals/emogrifier/runs/372326499
Also, an example I found is using composer.lock for the hash rather than composer.json.
There was a problem hiding this comment.
This jobs does not use a PHP version matrix. So adding it PHP matrix version to the cache key indeed did not make any sense. (It will do, though, for jobs with a version matrix, e.g., the PHP linting or the unit tests.) Good catch!
There was a problem hiding this comment.
We cannot hash composer.lock because we don't have one, and it gets generated only in the next step (composer install).
There was a problem hiding this comment.
I have added the explicit PHP version using a matrix now.
There was a problem hiding this comment.
I'll look into the restore-keys to find out what's the difference to key.
There was a problem hiding this comment.
This seems to be the official way AFAICT.
This should speed up the build and reduce our hassle with Travis CI. Part of #818.
oliverklee
force-pushed
the
task/actions-sniffer
branch
from
fc28440 to
e09c282
Compare
|
I have repushed. |
I presume this was a GitHub config change, as I don't spot anything different except that the PHP version now appears in the job commands. Thinking about this, I'm not sure that the cache key needs to reflect either PHP version or the contents of However, I'd be happy to approve this as it is (as you say it seems to be the recommended way), and we can revisit it later if we discover it is posisble to make caching more effective... - let me know what you think. |
I added the PHP version matrix.
I honestly don't know. I've read that some people had problems with the composer cache.
Yes, please. |
This should speed up the build and reduce our hassle with Travis CI.
Part of #818.