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

Make generic model compatible with PHP 8.1 #11846

Merged
merged 2 commits into from
Oct 13, 2022
Merged

Conversation

BafS
Copy link
Contributor

@BafS BafS commented Aug 3, 2022

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Add #[\ReturnTypeWillChange] (https://php.watch/versions/8.1/ReturnTypeWillChange) to avoid deprecation notices.

Fixes #11820

cc. @dkarlovi and @mandrean

Sorry, something went wrong.

Add #[\ReturnTypeWillChange] (https://php.watch/versions/8.1/ReturnTypeWillChange) to avoid deprecation notices.
@EgorBurykin
Copy link

Hello, is there any way we can have it merged? It seems to be a valid workaround.

@mbouwer
Copy link

mbouwer commented Sep 2, 2022

This would certainly help us. The PR seems valid, could this be merged into the codebase please?

@BafS
Copy link
Contributor Author

BafS commented Oct 3, 2022

@dkarlovi and @mandrean, would it be possible to merge this PR?

@BafS
Copy link
Contributor Author

BafS commented Oct 3, 2022

or @gracekarina maybe

@deguif
Copy link

deguif commented Oct 3, 2022

Any news on this one, many projects are waiting to support PHP 8.1 as without this, it triggers deprecations.

@BafS
Copy link
Contributor Author

BafS commented Oct 3, 2022

@HugoMario maybe you could help me to merge the PR? The PHP maintainers do not seems to reply. Thanks in advance!

@deguif
Copy link

deguif commented Oct 4, 2022

ping @frantuma maybe

@BafS
Copy link
Contributor Author

BafS commented Oct 5, 2022

@char0n @gracekarina @ponelat @frantuma could anyone help us to merge this PR?

@deguif
Copy link

deguif commented Oct 7, 2022

@BafS in the meantime you can rebase the branch so that This branch is out-of-date with the base branch message disappears.

Let's hope that a core contributor will be able to have a look and merge this one soon as it's a real pain currently for PHP >= 8.1 and will become more and more annoying as PHP 8.2 is near to be released.

@ponelat
Copy link
Member

ponelat commented Oct 10, 2022

Thanks for your patience folks, from my limited knowledge of PHP, ReturnTypeWillChange is backwards compatible with older versions of PHP? If so, I think it should be good to merge. @HugoMario if you're happy, I think lets get this merged.
And yes please, could you rebase @BafS

@deguif
Copy link

deguif commented Oct 11, 2022

@ponelat thanks for your feedback.
You're right, #[\ReturnTypeWillChange] is backward compatible as attributes are not interpreted on PHP versions lower than 8.x.

@BafS
Copy link
Contributor Author

BafS commented Oct 12, 2022

@ponelat Thanks for your answer! PR synced (not sure why this is necessary as there is no conflicts), and yes as @deguif says, it is fully backward compatible.

@ponelat ponelat requested a review from HugoMario October 12, 2022 11:40
@frantuma frantuma merged commit 621a733 into swagger-api:master Oct 13, 2022
@frantuma
Copy link
Member

Thanks!

@BafS
Copy link
Contributor Author

BafS commented Oct 14, 2022

Thank you! Do you know when the next release will be tagged?

@BafS
Copy link
Contributor Author

BafS commented Nov 2, 2022

@frantuma any chance to create a new tag to have the release available to the world?

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.

[PHP] ArrayAccess methods in Models missing return types
6 participants