Skip to content

Add GitHub actions #122

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

Merged
merged 9 commits into from
Jan 3, 2020
Merged

Add GitHub actions #122

merged 9 commits into from
Jan 3, 2020

Conversation

sagikazarmark
Copy link
Member

@sagikazarmark sagikazarmark commented Dec 30, 2019

Closes #119

@sagikazarmark sagikazarmark requested review from dbu and Nyholm December 30, 2019 14:36
@sagikazarmark
Copy link
Member Author

Do we agree with the style changes in this PR? I just used symfony, minus the rule mentioned in #119

Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

I agree with these style changes.

@dbu
Copy link
Contributor

dbu commented Jan 2, 2020

whats up with travis-ci?

@sagikazarmark
Copy link
Member Author

I removed the travis config, so it won't build this branch (and master after merging).

@dbu If you agree with the changes we can merge this PR and disable the required check for travis.

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

first time i see github workflow... quite verbose this thing, i quite liked the almost-bashscript thing of travis-ci :-(

but this looks correct to me, lets do these changes.

@@ -0,0 +1,337 @@
parameters:
Copy link
Contributor

Choose a reason for hiding this comment

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

great idea to add phpstan! and i agree with recording the baseline rather than trying to fix all in here.

we should open an issue to look into this file and see what we can fix / mark things as intentional.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

extensions: mbstring

- name: Setup Problem Matchers for PHPUnit
run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

for my understanding: what does this line thing do?

Copy link
Member Author

Choose a reason for hiding this comment

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

When tests fail, the output is parsed by GH actions and failures are listed separately from the build log. This helps parsing PHPUnit failures.

@dbu
Copy link
Contributor

dbu commented Jan 3, 2020

is this a new problem or general about php 7.4? https://github.com/php-http/message/pull/122/checks?check_run_id=367925137

@sagikazarmark
Copy link
Member Author

is this a new problem or general about php 7.4?

What do you mean exactly? It's a successful check.

@sagikazarmark sagikazarmark merged commit 2c1307e into master Jan 3, 2020
@sagikazarmark sagikazarmark deleted the github-actions branch January 3, 2020 15:02
@dbu
Copy link
Contributor

dbu commented Jan 4, 2020

https://github.com/php-http/message/pull/122/checks?check_run_id=367925137
What do you mean exactly? It's a successful check.

when you open the "Run tests" bit, it is full of warnings:

PHP Warning:  require_once(): PhpSpec\Loader\StreamWrapper::stream_set_option is not implemented! in /home/runner/work/message/message/vendor/phpspec/phpspec/src/PhpSpec/Loader/ResourceLoader.php on line 52
PHP Stack trace:
PHP   1. {main}() /home/runner/work/message/message/vendor/phpspec/phpspec/bin/phpspec:0
PHP   2. PhpSpec\Console\Application->run() /home/runner/work/message/message/vendor/phpspec/phpspec/bin/phpspec:26
PHP   3. PhpSpec\Console\Application->doRun() /home/runner/work/message/message/vendor/symfony/console/Application.php:148
PHP   4. PhpSpec\Console\Application->doRun() /home/runner/work/message/message/vendor/phpspec/phpspec/src/PhpSpec/Console/Application.php:104
PHP   5. PhpSpec\Console\Application->doRunCommand() /home/runner/work/message/message/vendor/symfony/console/Application.php:255
PHP   6. PhpSpec\Console\Command\RunCommand->run() /home/runner/work/message/message/vendor/symfony/console/Application.php:1000
PHP   7. PhpSpec\Console\Command\RunCommand->execute() /home/runner/work/message/message/vendor/symfony/console/Command/Command.php:255
PHP   8. PhpSpec\Loader\ResourceLoader->load() /home/runner/work/message/message/vendor/phpspec/phpspec/src/PhpSpec/Console/Command/RunCommand.php:168

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.

3 participants