Skip to content

Add GitHub Actions workflow #740

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.

Sign up for GitHub

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

Jump to bottom
Merged
nikic merged 1 commit into from
Dec 8, 2020
Merged

Add GitHub Actions workflow #740

nikic merged 1 commit into from
Dec 8, 2020

Conversation

@mglaman
Copy link
Contributor

@mglaman mglaman commented Dec 8, 2020 •
edited
Loading

Fixes #737

I didn't delete .travis.yml in this PR. I figured it could linger or be a follow-up post merge.

@mglaman mglaman force-pushed the gh-actions branch 10 times, most recently from bfc2432 to e930e07 Compare December 8, 2020 02:26
@mglaman
Copy link
Contributor Author

mglaman commented Dec 8, 2020

:/ hm, so test_old/run-php-src.sh 8.0.0 keeps erroring with

Fatal error: Uncaught Error: Undefined constant 'T_DOUBLE_COLON' in /home/runner/work/PHP-Parser/PHP-Parser/lib/PhpParser/Lexer.php:476
Stack trace:
#0 /home/runner/work/PHP-Parser/PHP-Parser/lib/PhpParser/Lexer.php(40): PhpParser\Lexer->createTokenMap()
#1 /home/runner/work/PHP-Parser/PHP-Parser/lib/PhpParser/Lexer/Emulative.php(44): PhpParser\Lexer->__construct(Array)
#2 /home/runner/work/PHP-Parser/PHP-Parser/test_old/run.php(136): PhpParser\Lexer\Emulative->__construct(Array)
#3 {main}
  thrown in /home/runner/work/PHP-Parser/PHP-Parser/lib/PhpParser/Lexer.php on line 476

https://github.com/mglaman/PHP-Parser/actions/runs/407264322

mglaman
mglaman commented Dec 8, 2020
View reviewed changes
.github/workflows/main.yml Outdated
Comment on lines 3 to 7
on:
push:
pull_request:
branches:
- "master"
Copy link
Contributor Author

@mglaman mglaman Dec 8, 2020

Choose a reason for hiding this comment

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

This way it runs on all branch bushes, but only PRs against master.

Copy link
Owner

@nikic nikic Dec 8, 2020

Choose a reason for hiding this comment

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

It is possible to make it run for all commits/PRs, regardless of branch? Other branches don't get used much right now, as there's currently only one supported version, but generally we'd want CI for everything.

Copy link
Contributor Author

@mglaman mglaman Dec 8, 2020

Choose a reason for hiding this comment

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

Cool; can do. This is something I've copied around – it runs on push to any branch but only PRs on master. I can make sure it runs on all things

mglaman
mglaman commented Dec 8, 2020
View reviewed changes
.github/workflows/main.yml
Comment on lines +28 to +29
# This is erroring, not sure why.
# - name: Coveralls
# run: "php vendor/bin/php-coveralls"
# if: ${{ success() }}
Copy link
Contributor Author

@mglaman mglaman Dec 8, 2020

Choose a reason for hiding this comment

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

This was failing: https://github.com/mglaman/PHP-Parser/runs/1514718327?check_suite_focus=true. Maybe due to a required secret.

@nikic
Copy link
Owner

nikic commented Dec 8, 2020

Is there something I need to do to make it run? Doesn't look like it runs in the PR.

nikic
nikic reviewed Dec 8, 2020
View reviewed changes
.github/workflows/main.yml Outdated
php-version: "8.0"
tools: composer:v2
- name: "Install PHP 8 dependencies"
run: "composer update --ignore-platform-req=php --no-progress --prefer-dist"
Copy link
Owner

@nikic nikic Dec 8, 2020

Choose a reason for hiding this comment

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

The ignore-platform-req is probably not needed anymore?

@mglaman
Copy link
Contributor Author

mglaman commented Dec 8, 2020

So I thought it would run on the PR, but not until it's merged. So it currently only runs on the branch push for my fork. After this is merged things would start running.

@mglaman
Copy link
Contributor Author

mglaman commented Dec 8, 2020

Re-run after suggestions addressed: https://github.com/mglaman/PHP-Parser/actions/runs/409233894

@nikic nikic merged commit 5e36ef7 into nikic:master Dec 8, 2020
@mglaman mglaman deleted the gh-actions branch December 8, 2020 22:01
@nikic
Copy link
Owner

nikic commented Dec 8, 2020

Looks like it does work fine once merged: https://github.com/nikic/PHP-Parser/actions/runs/409258444

The T_DOUBLE_COLON issue sounds like ext-tokenizer isn't enabled. That's probably due to the -n flag in

PHP-Parser/test_old/run-php-src.sh

Line 5 in 5e36ef7

php -n test_old/run.php --verbose --no-progress --php-version=$VERSION PHP ./data/php-src
. IIRC that flag is there to disable xdebug, but with the new setup it can probably just be dropped?

@nikic
Copy link
Owner

nikic commented Dec 8, 2020

Dropped the -n flag in 0f64504, build is green now.

Only remaining question is the coverage generation.

@nikic
Copy link
Owner

nikic commented Dec 8, 2020

Possibly need to specify COVERALLS_REPO_TOKEN: https://github.com/php-coveralls/php-coveralls#github-actions

@nikic
Copy link
Owner

nikic commented Dec 8, 2020

Coverage enabled in 7c09e09 and Travis dropped in bec74ac. Looks like everything is working fine!

@mglaman
Copy link
Contributor Author

mglaman commented Dec 8, 2020

Awesome!

@TomasVotruba
Copy link
Contributor

TomasVotruba commented Dec 9, 2020

@mglaman Thank you 👍

jkuchar added a commit to grifart/scaffolder that referenced this pull request Jan 18, 2021
@jkuchar
tests: run with ext-tokenizer enabled (source: nikic/PHP-Parser#740 (…
84e8123 
…comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikic nikic nikic left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Migrate from Travis to GitHub Actions

3 participants

@mglaman @nikic @TomasVotruba