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

[TASK] Move the PHP code sniffing to GitHub actions #832

Merged
merged 1 commit into from
Jan 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# https://help.github.com/en/categories/automating-your-workflow-with-github-actions

on:
- pull_request
- push

name: CI

jobs:
php-code-sniffer:
name: PHP Code Sniffer

runs-on: ubuntu-latest

strategy:
matrix:
php-version:
- 7.3

steps:
- name: Checkout
uses: actions/checkout@v1

- name: "Cache dependencies installed with composer"
uses: actions/cache@v1
with:
path: ~/.composer/cache
key: php${{ matrix.php-version }}-composer-${{ hashFiles('**/composer.json') }}
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot hash composer.lock because we don't have one, and it gets generated only in the next step (composer install).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the explicit PHP version using a matrix now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look into the restore-keys to find out what's the difference to key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to be the official way AFAICT.

restore-keys: |
php${{ matrix.php-version }}-composer-

- name: Install Composer dependencies
run: composer install --no-progress

- name: Run PHP Code Sniffer
run: composer ci:php:sniff
9 changes: 0 additions & 9 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,6 @@ script:
echo "Skipping Psalm (will only be run on PHP 7.3).";
fi;

- >
if is_php_73; then
echo;
echo "Running PHP_CodeSniffer";
composer ci:php:sniff;
else
echo "Skipping PHP_CodeSniffer (will only be run on PHP 7.3).";
fi;

- >
if is_php_73; then
echo;
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
([#776](https://github.com/MyIntervals/emogrifier/pull/776))

### Changed
- Move the code sniffing from Travis CI to GitHub actions
([#832](https://github.com/MyIntervals/emogrifier/pull/830)
- Upgrade to Symfony 5.0
([#822](https://github.com/MyIntervals/emogrifier/pull/820)
- Clean up the folder structure and autoloading configuration
Expand Down