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

[PropertyInfo] Deprecate PropertyInfo Type #53160

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Dec 20, 2023

Q A
Branch? 7.1
Bug fix? no
New feature? no
Deprecations? yes
Issues
License MIT

This PR is a follow-up of #52510.

As the TypeInfo's Type aims to represent types in the Symfony ecosystem, the PropertyInfo's Type needs to be deprecated in favor of the first one.

@mtarld mtarld requested a review from dunglas as a code owner December 20, 2023 14:50
@carsonbot carsonbot added this to the 7.1 milestone Dec 20, 2023
@carsonbot carsonbot changed the title [TypeInfo][PropertyInfo] Deprecate PropertyInfo Type [PropertyInfo] [TypeInfo] Deprecate PropertyInfo Type Dec 20, 2023
@mtarld mtarld force-pushed the chore/deprecate-property-info-type branch 5 times, most recently from 6801f87 to 559105e Compare December 26, 2023 15:40
@mtarld mtarld force-pushed the chore/deprecate-property-info-type branch 2 times, most recently from 12f79da to e6740da Compare December 27, 2023 09:27
@mtarld mtarld requested a review from chalasr December 27, 2023 09:47
@mtarld mtarld force-pushed the chore/deprecate-property-info-type branch from e6740da to 2ebbf09 Compare December 27, 2023 14:16
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Looking good 👍

@mtarld mtarld force-pushed the chore/deprecate-property-info-type branch 5 times, most recently from db0f17e to 7497a94 Compare January 2, 2024 13:33
@mtarld mtarld force-pushed the chore/deprecate-property-info-type branch 2 times, most recently from 419469f to 4f106ba Compare January 5, 2024 10:13
@mtarld mtarld force-pushed the chore/deprecate-property-info-type branch from da8502d to 6b68ad8 Compare March 30, 2024 15:37
@dunglas
Copy link
Member

dunglas commented Mar 30, 2024

@mtarld as discussed at SymfonyLive, could you also try to update API Platform to use this patch? As we do extensive use of PropertyInfo (the component has initially be created as part of API Platform), this will validate that all use cases are covered and that we have a good migration path. Thanks!

@mtarld mtarld force-pushed the chore/deprecate-property-info-type branch 4 times, most recently from bf280d3 to b4c7e67 Compare March 31, 2024 10:04
@mtarld
Copy link
Contributor Author

mtarld commented Apr 2, 2024

@dunglas, I tried to use the type from TypeInfo component within API Platform, and so far I don't see any blocker. Indeed, the same BC/deprecation tricks in this PR can be done as well in API Platform.

But, it is plenty of work, I'll try to send a draft PR soon.

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Much better.
I think this should make it in 7.1 so that we have at least one usage of the new type-info component internally, and we can iterate.

@chalasr chalasr added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Apr 3, 2024
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Let's see how this goes :)

Co-authored-by: Baptiste Leduc <baptiste.leduc@gmail.com>
@mtarld mtarld force-pushed the chore/deprecate-property-info-type branch from b4c7e67 to d32e81c Compare April 4, 2024 06:15
@fabpot
Copy link
Member

fabpot commented Apr 5, 2024

Thank you so much @mtarld, great work here

@fabpot fabpot merged commit 19affe1 into symfony:7.1 Apr 5, 2024
9 of 10 checks passed
@mtarld mtarld deleted the chore/deprecate-property-info-type branch April 5, 2024 06:49
@fabpot fabpot mentioned this pull request May 2, 2024
xabbuh added a commit that referenced this pull request May 15, 2024
…acts (xabbuh)

This PR was merged into the 7.1 branch.

Discussion
----------

[PropertyInfo] remove no longer needed deprecation contracts

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

the dependency was introduced in #53160, but is no longer required since we reverted the deprecations in #54789

Commits
-------

18296cd remove no longer needed deprecation contracts
xabbuh added a commit that referenced this pull request May 17, 2024
…ue" on string" (michaljusiega, xabbuh)

This PR was merged into the 7.1 branch.

Discussion
----------

[Serializer] Fixed "Warning: Attempt to read property "value" on string"

| Q             | A
| ------------- | ---
| Branch?       | 7.1 (7.1.0-BETA1)
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fixed regresion of #53160
| License       | MIT

Hi! I've updated my project to `7.1.0-BETA1` and I found an error.

After update I got: `Warning: Attempt to read property "value" on string` in `vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:748` because in `733 line` the enum-case of value `TypeIdentifier` has been already readed.

![obraz](https://github.com/symfony/symfony/assets/16488888/5227ac76-0fb0-4b00-ab68-2394ab41d7b7)

Quite not sure if tests are possible here.

Commits
-------

90251c9 add test
4b3dcf1 Fixed "Warning: Attempt to read property "value" on string"
symfony-splitter pushed a commit to symfony/serializer that referenced this pull request May 17, 2024
…ue" on string" (michaljusiega, xabbuh)

This PR was merged into the 7.1 branch.

Discussion
----------

[Serializer] Fixed "Warning: Attempt to read property "value" on string"

| Q             | A
| ------------- | ---
| Branch?       | 7.1 (7.1.0-BETA1)
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fixed regresion of symfony/symfony#53160
| License       | MIT

Hi! I've updated my project to `7.1.0-BETA1` and I found an error.

After update I got: `Warning: Attempt to read property "value" on string` in `vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:748` because in `733 line` the enum-case of value `TypeIdentifier` has been already readed.

![obraz](https://github.com/symfony/symfony/assets/16488888/5227ac76-0fb0-4b00-ab68-2394ab41d7b7)

Quite not sure if tests are possible here.

Commits
-------

90251c9361 add test
4b3dcf1ca6 Fixed "Warning: Attempt to read property "value" on string"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecation PropertyInfo ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" Status: Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants