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

php-token-bucket: replace Travis with GH Actions #3

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Conversation

davidrans
Copy link
Member

qa_req 0

Comment on lines +4 to +9
push:
branches:
- '**'
pull_request:
branches:
- '**'
Copy link
Member

Choose a reason for hiding this comment

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

We want to run this on all pushes?

Suggested change
push:
branches:
- '**'
pull_request:
branches:
- '**'
push:
pull_request:

Copy link
Member Author

Choose a reason for hiding this comment

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

It's fast, so I don't see why not

Copy link
Member Author

Choose a reason for hiding this comment

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

It was helpful when debugging which is why I did it. But if it makes more sense to make it PR only, that's fine

Comment on lines 21 to 30
- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.3'

- name: Run composer install
run: composer install

- name: Run psalm
run: ./vendor/bin/psalm --output-format=github --update-baseline
Copy link
Member

Choose a reason for hiding this comment

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

Think all this could be replaced with https://github.com/psalm/psalm-github-actions

Comment on lines +4 to +9
push:
branches:
- '**'
pull_request:
branches:
- '**'
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

Comment on lines +19 to +28
- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: '8.3'

- name: Install dependencies
run: composer install

- name: Run tests
run: vendor/bin/phpunit tests
Copy link
Member

Choose a reason for hiding this comment

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

Could be replaced with https://github.com/php-actions/phpunit I think.

mlahargou
mlahargou previously approved these changes Aug 20, 2024
Copy link
Member

@mlahargou mlahargou left a comment

Choose a reason for hiding this comment

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

CR 📱 with a few minor comments/questions.

Co-authored-by: Michael Lahargou <[email protected]>
@mlahargou
Copy link
Member

CR 📱

@davidrans
Copy link
Member Author

Those are all good suggestions, but I'm just gonna merge this cause we're probably not going to change it again for a while

@davidrans davidrans merged commit 9f696bb into master Aug 21, 2024
4 checks passed
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