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

Fix the build pipeline in GitHub Actions #191

Merged
merged 2 commits into from
Nov 26, 2022

Conversation

mpdude
Copy link
Contributor

@mpdude mpdude commented Nov 24, 2022

This hopefully makes the builds all 🟩 again.

Many changes were necessary to fix the Coding Standards check. See comments below for explanations of particular other changes.


- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
coverage: none
ini-values: "memory_limit=-1"
ini-values: "memory_limit=-1, zend.assertions=1"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to be sure... this was not set in my local dev box, causing all tests to pass since assert() was not evaluated.


- name: Install dependencies
run: |
composer config --no-plugins allow-plugins.dealerdirect/phpcodesniffer-composer-installer true
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 guess this is due to recent changes in Composer.

run: composer test

psalm:
name: Run Psalm
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 don't think we need to run this in all matrix versions 🌱 . When PHP 7.4 and its available dependencies are fine, the other versions should be as well.

@@ -24,7 +24,7 @@
"friends-of-behat/mink-extension": "^2.5",
"friends-of-behat/page-object-extension": "^0.3.2",
"friends-of-behat/service-container-extension": "^1.1",
"sylius-labs/coding-standard": "^4.1.1",
"sylius-labs/coding-standard": ">=4.1.1, <=4.2.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

They changed something with their config file format, so I had to limit the version range to keep things working over the broad range of PHP versions tested here.

@@ -48,6 +48,10 @@ Feature: Autowiring contexts

App\Tests\SomeContext:
public: true

# https://github.com/symfony/symfony/pull/35879/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addresses a Symfony 5.2 (IIRC?) deprecation + failure as of 6.0.

@mpdude
Copy link
Contributor Author

mpdude commented Nov 24, 2022

@Yozhef I know you have a lot of other, more important things to worry about 😢 .

Maybe if you could approve my workflows to run – this might be an easy merge, and would get the builds back to 🟢 .

@mpdude
Copy link
Contributor Author

mpdude commented Nov 24, 2022

Or maybe @pamil you can approve my workflows to run (needs manual approval for first-time contribution).

@Yozhef
Copy link
Contributor

Yozhef commented Nov 24, 2022

@mpdude I will look at everything tomorrow and write back.

@Yozhef Yozhef merged commit 3645bd6 into FriendsOfBehat:master Nov 26, 2022
@mpdude
Copy link
Contributor Author

mpdude commented Nov 26, 2022

Thank you @Yozhef!

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