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

Improve CI configuration #619

Merged
merged 5 commits into from
Jan 5, 2021
Merged

Improve CI configuration #619

merged 5 commits into from
Jan 5, 2021

Conversation

t0mmy742
Copy link
Contributor

@t0mmy742 t0mmy742 commented Dec 30, 2020

Multiple small things here:

  • Change the way we are using action/cache from GitHub Actions. We were caching vendor directory, which is not really a good practice as mentioned here. Moreover, cache is now detecting changes of composer.lock, since it used an hash of this file for generating the key.
  • I reorganized GitHub Actions workflows. I added PHPUnit tests to PHP 8, in addition of PHP 7.4.
  • PHPBench updated to 1.0.0-alpha4. Previous versions does not support PHP 8. It does not seem to change anything for us. We should be ready when a stable version get released.
  • vfsstream minimal version is now 1.6.7. Released last year, this version corrected a deprecated feature in PHP 7.4, removed in PHP 8.0 (see here). Last version is 1.6.8.

Copy link
Collaborator

@Slamdunk Slamdunk left a comment

Choose a reason for hiding this comment

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

PR looks ok, just one revert IMHO 👍

.github/workflows/benchmarks.yml Show resolved Hide resolved
Copy link
Sponsor Collaborator

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Ocramius
Copy link
Sponsor Collaborator

Ah, I think the required checks need to be re-configured for this to be merged 😬

@Ocramius
Copy link
Sponsor Collaborator

These pipelines no longer exist:

  • PHPUnit tests on PHP 8 (locked, 8.0, ubuntu-latest)
  • Run benchmarks (locked, 7.4, ubuntu-latest)

These changes are OK, but require @lcobucci to change repo settings before merge

@lcobucci
Copy link
Owner

Thanks folks! I'm taking a break from github and my laptop for a couple of days. Will take a look at it around the weekend.

Copy link
Owner

@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

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

Just some tiny things, thanks for the help @t0mmy742!

.github/workflows/phpunit.yml Show resolved Hide resolved
.github/workflows/benchmarks.yml Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
@lcobucci
Copy link
Owner

lcobucci commented Jan 2, 2021

@t0mmy742 did you revert the cache optimisation on purpose? I thought the feedback was mostly about the templating.

@t0mmy742
Copy link
Contributor Author

t0mmy742 commented Jan 2, 2021

@lcobucci I missed something with git rebase 😢
I'm going to correct this

@t0mmy742 t0mmy742 marked this pull request as draft January 2, 2021 15:23
@lcobucci
Copy link
Owner

lcobucci commented Jan 2, 2021

@lcobucci I missed something with git rebase
I'm going to correct this

It's alright, happens to everyone 😆 That's why the reflog is there for

@t0mmy742 t0mmy742 marked this pull request as ready for review January 2, 2021 16:25
@t0mmy742 t0mmy742 requested a review from lcobucci January 2, 2021 16:27
t0mmy742 and others added 5 commits January 5, 2021 22:16
Bumping the minimum to ensure PHP 8.0 compatibility.
Avoiding caching the vendor directory and using the lock file to
calculate the hash, according to the best practices of the action we're
using.

More info: https://github.com/shivammathur/setup-php#cache-composer-dependencies
Adding checks on PHP 8.1 as well.
@lcobucci lcobucci merged commit cf2da1e into lcobucci:4.1.x Jan 5, 2021
@lcobucci lcobucci self-assigned this Jan 5, 2021
@lcobucci lcobucci added the CI label Jan 5, 2021
@lcobucci lcobucci added this to the 4.1.0 milestone Jan 5, 2021
@lcobucci lcobucci changed the title GitHub action no cache vendor Improve CI configuration Jan 5, 2021
@lcobucci
Copy link
Owner

lcobucci commented Jan 5, 2021

@t0mmy742 thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants