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

Migrate codebase to PHP 8.1 #94

Merged
merged 1 commit into from
Dec 30, 2022
Merged

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Dec 29, 2022

Let's give this little library its PHP 8.1 treatment. If this PR gets accepted, I'll create a 2.0.x branch and merge it there.

Replaces #44

@derrabus derrabus added this to the 2.0.0 milestone Dec 29, 2022
Comment on lines +32 to +35
private const SERIALIZATION_FORMAT_USE_UNSERIALIZER = 'C';
private const SERIALIZATION_FORMAT_AVOID_UNSERIALIZER = 'O';
Copy link
Member Author

Choose a reason for hiding this comment

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

Those constants are never used publicly and I believe they should not be part of the public interface. Do we need to flag them as deprecated on 1.x or is it enough to just switch them to private in 2.0?

Copy link
Member

Choose a reason for hiding this comment

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

Let's flag them as deprecated out of consistency with doctrine/orm#10342?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in #96

@derrabus derrabus changed the base branch from 1.4.x to 2.0.x December 30, 2022 00:16
@derrabus derrabus marked this pull request as ready for review December 30, 2022 00:18
@derrabus derrabus merged commit c622228 into doctrine:2.0.x Dec 30, 2022
@derrabus derrabus deleted the bump/php-8.1 branch December 30, 2022 00:23
@@ -31,37 +31,33 @@ final class Instantiator implements InstantiatorInterface
*
* @deprecated This constant will be private in 2.0
Copy link
Member

Choose a reason for hiding this comment

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

you forgot to remove that @deprecated tag

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. Wanna submit a PR?

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.

3 participants