-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Enhancement: Switch to using GitHub actions #1847
Conversation
b2ca95d
to
1e9ecf2
Compare
php: | ||
- 7.2 | ||
- 7.3 | ||
- nightly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't run on nightly
- do we care?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No really if you ask me. Isnt there a 7.4 image available already since release is in 2 weeks from now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is available now, yes!
- name: "Install PHP with extensions" | ||
uses: shivammathur/[email protected] | ||
with: | ||
php-version: 7.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we running phpcs only for 7.1 at this point? No idea if we could actually have a diff there based on a PHPversion. Dont really think so though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it suffices to run a coding standards fixer on the lowest supported version of PHP.
@@ -0,0 +1,76 @@ | |||
# https://help.github.com/en/categories/automating-your-workflow-with-github-actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me just add a comment on rootlevel. Isnt it maybe wise to integrate all tooling already that we want to have implemented? Like PHPstan and maybe some comp. checks or will we add them later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to add them later, one by one.
52c0b13
to
93ff7c3
Compare
Looks like Actions aren't enabled - at least they are not running and there are no complaints about invalid YML configuration either. Can you double-check that GitHub Actions are enable? Could also be that they are not activated because I do not have push privileges. |
93ff7c3
to
8b80f88
Compare
8b80f88
to
3d749eb
Compare
Yes, actions are enabled... Dunno what's wrong. |
I see that there is now something within actions, so maybe if we rebase this it should be fine i guess |
Will do! |
1a8f5c3
to
7d0316a
Compare
Looking good! |
ca3a0d6
to
e0c009a
Compare
I need to collect code coverage here as well, give me a moment! |
e0c009a
to
46a75b9
Compare
|
||
- name: "Send code coverage report to Codecov.io" | ||
env: | ||
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This token needs to be added via https://github.com/fzaninotto/faker/settings/secrets.
Also take note of
8ca9d4f
to
504197e
Compare
504197e
to
e21e79b
Compare
Please avoid force pushing. The merger can squash the commits when merging the PR. |
I think this is good for another review! |
Please make sure to squash the commits on merge! |
Thank you, @fzaninotto and @pimjansen! |
This PR