search

php-actions / php-build

Fast builds for repositories using php-actions.
9 stars 9 forks source link

Allow PHP Versions to Update #17

Closed westy92 closed 5 months ago

westy92 commented 5 months ago

Fixes https://github.com/php-actions/php-build/issues/16.

@g105b is there an easy way to test this?

westy92 commented 5 months ago

@g105b I believe I was able to prove it works here: https://github.com/westy92/holiday-event-api-php/actions/runs/8841138041

westy92 commented 5 months ago

Should we bump php_build_version="build2.1.1" to 2.1.2?

g105b commented 5 months ago

Looks good to me, thanks so much for this PR.

The only thing I can't think of how to test is what happens to the runner when it runs again and there's a new release of PHP. I don't think there's a straightforward way of testing this without waiting for a new release and crossing our fingers that the functionality works as expected.

Do you have any other projects that haven't been built in a while that this can be tested with, just for clarity?

Should we bump php_build_version="build2.1.1" to 2.1.2?

Yes please. This is the only mechanism I am aware of that can be used to cache the actual php_build process, so when a new workflow starts it doesn't have to build PHP each time. As long as this string changes, it will invalidate the cache.

westy92 commented 5 months ago

Thank you for the review! I bumped the version. Unfortunately the only project I have using this I already tested with.

g105b commented 5 months ago

I think your fix is identifying an issue that is worse to keep in the repo than it would be if it caused a potential minor failure in the future, so I think it makes sense to merge directly. I have a lot of repositories that use this action every day, so I'll notice any issues straight away. Do you agree? If so I'll merge tonight.

westy92 commented 5 months ago

Yes I agree!

westy92 commented 5 months ago

I updated the version to 2.2.1 since 2.2.0 has already been tagged and released but never updated here. I opened two other PRs that you can see referenced above.

Thank you for reviewing this! Make sure to tag v2.2.1 and v2 after merging. :)

g105b commented 5 months ago

Cheers! I'll make the release now.