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

GH Actions: run against PHP 8.3 #11601

Merged
merged 2 commits into from
Aug 30, 2023

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 28, 2023

Re: branch selection: as Composer 2.2 is marked as an LTS release, by rights, this PR should be pulled to the 2.2 branch, but the 2.2 branch isn't even being tested against PHP 8.2, which gives me the impression that the "LTS" is only about Composer native functionality, not about PHP version support.

Might be a good idea to clarify this somewhere ?


GH Actions: run against PHP 8.3

What with PHP 8.3 being close to the first RC, I'd like to suggest enabling runs against PHP 8.3 for the linting and test runs.

  • Linting passes on PHP 8.3, so I propose to not allow new failures to be introduced there.
  • The test runs, however, do not pass against PHP 8.3, so I'm marking those as experimental for now to allow for fixing the issue(s).

As for the compatibility issues (based on the test runs):

@stof
Copy link
Contributor

stof commented Aug 28, 2023

The test should be changed to do new ReflectionMethod(ComposerRepository::class, 'whatProvides') instead of new ReflectionMethod($repo, 'whatProvides') (as $repo is a a mock so its class is a child class of ComposerRepository)

@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 28, 2023

The test should be changed to do new ReflectionMethod(ComposerRepository::class, 'whatProvides') instead of new ReflectionMethod($repo, 'whatProvides') (as $repo is a a mock so its class is a child class of ComposerRepository)

@stof Good point, my brain clearly wasn't working. I've opened PR #11602 to fix this.

@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 28, 2023

Once PR #11599 and #11602 have been merged, I could rebase this PR and remove the experimental: true (or explicitly set it to false), if so desired.

@Seldaek
Copy link
Member

Seldaek commented Aug 30, 2023

Happy to get a rebase here, and yes experimental false sounds ok if it builds, now that 8.3 is in RC I guess it shouldn't have surprises anymore.

- php-version: "8.3"
dependencies: highest-ignore
os: ubuntu-latest
experimental: true
Copy link
Member

Choose a reason for hiding this comment

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

Sorry these in fact should replace the 8.2 above at line 58 and 62, instead of adding new builds. And then at line 33 it needs to be added as well to get the regular locked build. That keeps the amount of builds to a reasonable-ish level. Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

To reduce the needs to update, should the lowest-ignore and highest-ignore jobs use latest as PHP version ?

What with PHP 8.3 being close to the first RC, I'd like to suggest enabling runs against PHP 8.3 for the linting and test runs.

* Linting passes on PHP 8.3, so I propose to not allow new failures to be introduced there.
* The test runs, however, do not pass against PHP 8.3, so I'm marking those as `experimental` for now to allow for fixing the issue(s).

As for the compatibility issues (based on the test runs):
* PR 11599 fixes all known deprecation notices.
* There is, however, one test failure, which I'm not exactly sure how to fix, so I'm leaving this for the maintainers to decide upon.
    Details:
    Prior to PHP 8.3, `ReflectionMethod` could set a `private` method on a parent class to accessible. This is no longer possible in PHP 8.3 since php/php-src 9470 and breaks the `Composer\Test\Repository\ComposerRepositoryTest::testWhatProvides` test.
    Also see: https://3v4l.org/8YcIk/rfc#vgit.master
.github/workflows/lint.yml Outdated Show resolved Hide resolved
@Seldaek Seldaek added this to the 2.6 milestone Aug 30, 2023
* Don't add PHP 8.3 to the `lint` workflow.
* Replace the PHP 8.2 extra builds instead of adding to them for `test`.
* Don't allow builds to fail.
@jrfnl jrfnl force-pushed the feature/ghactions-add-php-8.3 branch from eb44b39 to 2db1797 Compare August 30, 2023 09:56
@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 30, 2023

@Seldaek Rebased and updated.

@Seldaek Seldaek merged commit bb1aa84 into composer:main Aug 30, 2023
20 checks passed
@Seldaek
Copy link
Member

Seldaek commented Aug 30, 2023

Thanks

@jrfnl jrfnl deleted the feature/ghactions-add-php-8.3 branch August 30, 2023 14:10
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.

None yet

3 participants