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

Enhancement: Run phpstan on GitHub Actions #28

Merged
merged 1 commit into from
Aug 3, 2020

Conversation

localheinz
Copy link
Contributor

This PR

  • runs phpstan on GitHub Actions

Follows #27.


runs-on: ubuntu-latest

strategy:
matrix:
php-version:
Copy link
Contributor Author

@localheinz localheinz Aug 3, 2020

Choose a reason for hiding this comment

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

By introducing a matrix strategy here, we can reference the resolved values using ${{ matrix.php-version }}.

Since we only run the static analysis for a single PHP version, it might seem that it does not make a lot of sense.

However, in my experience, it's quite useful, as we can reference the ${{ matrix.php-version }} in multiple places without having to hard-code the actual PHP version.

Also note how the matrix values are appended to the job name:

Screen Shot 2020-08-03 at 19 11 27

💁‍♂️ For reference, see:

strategy:
matrix:
php-version:
- 7.2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we decide to run the static analysis on PHP 7.4, we only need to change the value here.

@nikolaposa nikolaposa self-assigned this Aug 3, 2020
- name: Echo a greeting
run: echo 'Hello, GitHub Actions!'
- name: Checkout
uses: actions/checkout@v2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to do something with the code, we need to check out the repository first.

💁‍♂️ For reference, see:

uses: actions/checkout@v2

- name: Install PHP
uses: shivammathur/setup-php@v2
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 could use one of the PHP binaries that are natively available instead.

However, these are currently limited to:

  • php (PHP 7.4.8)
  • php7.1 (PHP 7.1.33)
  • php7.2 (PHP 7.2.32)
  • php7.3 (PHP 7.3.20)
  • php7.4 (PHP 7.4.8)

Additionally, a lot of extensions are installed, but not all (!) of them.

Instead, a lot of people use shivammathur/setup-php to set up a wider (!) range of PHP versions, along with code coverage (if need be) and desired extensions.

Shivam Mathur does a great job in maintaining this action, and is very responsive.

💁‍♂️ For reference, see:


- name: Determine composer cache directory
id: determine-composer-cache-directory
run: echo "::set-output name=directory::$(composer config cache-dir)"
Copy link
Contributor Author

@localheinz localheinz Aug 3, 2020

Choose a reason for hiding this comment

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

Steps can have outputs - for example, it is possible to define output parameters (like here) or environment variables.

Here we determine the composer cache directory, instead of hard-coding it, so we can reference it later.

💁‍♂️ For reference, see:

run: echo "::set-output name=directory::$(composer config cache-dir)"

- name: Cache dependencies installed with composer
uses: actions/cache@v2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With actions/cache we can cache arbitrary directories or files between jobs.

💁‍♂️ For reference, see:

- name: Cache dependencies installed with composer
uses: actions/cache@v2
with:
path: ${{ steps.determine-composer-cache-directory.outputs.directory }}
Copy link
Contributor Author

@localheinz localheinz Aug 3, 2020

Choose a reason for hiding this comment

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

Here we pick up the output parameter again to specify a value for the path input parameter.

Notice how we use the step identifier determine-composer-cache-directory to reference the value of the directory output of the corresponding step.

💁‍♂️ For reference, see:

uses: actions/cache@v2
with:
path: ${{ steps.determine-composer-cache-directory.outputs.directory }}
key: php-${{ matrix.php-version }}-composer-${{ hashFiles('composer.json') }}
Copy link
Contributor Author

@localheinz localheinz Aug 3, 2020

Choose a reason for hiding this comment

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

Here we specify a value for the key input parameter, the cache key to use for caching the contents of the composer cache directory.

Notice how we use the ${{ matrix.php-version }} expression which resolves to the corresponding value in the build matrix. Here it will resolve to 7.2.

Also, we use the hashfiles() function to calculate a hash from composer.json as part of the cache key.

💁‍♂️ For reference, see:


- name: Cache dependencies installed with composer
uses: actions/cache@v2
with:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the with key, we can specify a map of parameter names to values that are accepted by an action.

💁‍♂️ For reference, see:

with:
path: ${{ steps.determine-composer-cache-directory.outputs.directory }}
key: php-${{ matrix.php-version }}-composer-${{ hashFiles('composer.json') }}
restore-keys: php-${{ matrix.php-version }}-composer-
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we can list one or more cache keys.

Note that the inputs for an action need to be strings. If we want to specify more items, we need to use a multi-line string:

diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml
index e7b00a0..07b7e54 100644
--- a/.github/workflows/build.yaml
+++ b/.github/workflows/build.yaml
@@ -38,7 +38,9 @@ jobs:
         with:
           path: ${{ steps.determine-composer-cache-directory.outputs.directory }}
           key: php-${{ matrix.php-version }}-composer-${{ hashFiles('composer.json') }}
-          restore-keys: php-${{ matrix.php-version }}-composer-
+          restore-keys: |
+            php-${{ matrix.php-version }}-composer-
+            foo-bar-baz

       - name: Install highest dependencies from composer.json
         run: composer install --no-interaction

Also note that the value for the key input is automatically used as a key, so here we only specify additional, optional keys (or prefixes).

💁‍♂️ For reference, see:

@localheinz localheinz marked this pull request as ready for review August 3, 2020 17:10
Copy link
Contributor Author

@localheinz localheinz left a comment

Choose a reason for hiding this comment

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

As you can see, the configuration is indeed a bit verbose.

Unfortunately, GitHub Actions does not yet support YAML anchors and references - both would allow to reduce duplication.

💁‍♂️ For reference, see:

@nikolaposa
Copy link
Owner

GitHub Actions does not yet support YAML anchors and references

I was just about to ask how we gonna reuse steps such as installing Composer dependencies between jobs. If duplication is inevitable, would it make sense to go with the approach of configuration file per job, so for example: coding-standards.yml, static-analysis.yml, tests.yml, etc.?

@localheinz
Copy link
Contributor Author

@nikolaposa

I was just about to ask how we gonna reuse steps such as installing Composer dependencies between jobs. If duplication is inevitable, would it make sense to go with the approach of configuration file per job, so for example: coding-standards.yml, static-analysis.yml, tests.yml, etc.?

That would be possible, but take note that there is no way to enforce dependencies between workflows.

On the other hand, it is possible to do so for jobs - that is, you can hold of running jobs in a workflow until other jobs have completed by using the needs directive.

Perhaps do so at a later time, when necessary?

💁‍♂️ For reference, see:

@nikolaposa
Copy link
Owner

Perhaps do so at a later time, when necessary?

Agreed. 👍

@nikolaposa nikolaposa self-requested a review August 3, 2020 18:31
@nikolaposa nikolaposa merged commit 464d83f into nikolaposa:master Aug 3, 2020
@localheinz
Copy link
Contributor Author

Thank you, @nikolaposa!

@localheinz localheinz deleted the feature/static-analysis branch August 3, 2020 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants