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

[FrameworkBundle][Validator] Deprecate annotation occurrences #51425

Merged

Conversation

alexandre-daubois
Copy link
Contributor

@alexandre-daubois alexandre-daubois commented Aug 18, 2023

Q A
Branch? 6.4
Bug fix? no
New feature? no
Deprecations? yes
Tickets Part of #51381
License MIT
Doc PR -
  • Deprecate framework.validation.enable_annotations in favor of framework.validation.enable_attributes
  • Deprecate framework.serializer.enable_annotations in favor of use framework.serializer.enable_attributes
  • Deprecate ValidatorBuilder::enableAnnotationMapping() in favor of ValidatorBuilder::enableAttributeMapping()
  • Deprecate ValidatorBuilder::disableAnnotationMapping() in favor of ValidatorBuilder::disableAttributeMapping()
  • Deprecate AnnotationLoader in favor of AttributeLoader

@stof
Copy link
Member

stof commented Aug 18, 2023

This should be in the same PR than #51426 instead of being a separate PR IMO, as this is about the FrameworkBundle semantic configuration related to the same change.
We don't require each PR to change only one of the packages. It is actually quite the opposite. The main reason to have a mono-repo is to allow to have one change spanning multiple packages.

@alexandre-daubois alexandre-daubois force-pushed the deprecate-enable-annotations branch 3 times, most recently from 97dc60e to 0bd8e1f Compare August 18, 2023 14:01
@alexandre-daubois alexandre-daubois force-pushed the deprecate-enable-annotations branch 3 times, most recently from 89a4e3c to 1723d35 Compare August 18, 2023 14:32
@alexandre-daubois
Copy link
Contributor Author

Addressed your comments @derrabus, thank you!

@derrabus
Copy link
Member

Thank you @alexandre-daubois.

@derrabus derrabus merged commit f354c1e into symfony:6.4 Aug 22, 2023
4 of 9 checks passed
@alexandre-daubois
Copy link
Contributor Author

Working on the clean-up 👍

@alexandre-daubois alexandre-daubois deleted the deprecate-enable-annotations branch August 22, 2023 13:30
derrabus added a commit that referenced this pull request Aug 23, 2023
…to AnnotationLoader (derrabus)

This PR was merged into the 6.4 branch.

Discussion
----------

[Validator] Un-deprecate passing an annotation reader to AnnotationLoader

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Follow-up to #51425
| License       | MIT
| Doc PR        | N/A

#51425 deprecated the `AnnotationLoader` class and its replacement already does not accept an annotation reader anymore. Thus, the deprecation of the constructor parameter is redundant, which is why I propose to take it back.

cc `@alexandre`-daubois

Commits
-------

4fa1d32 [Validator] Un-deprecate passing an annotation reader to AnnotationLoader
nicolas-grekas added a commit that referenced this pull request Aug 23, 2023
…ns (alexandre-daubois)

This PR was merged into the 7.0 branch.

Discussion
----------

[FrameworkBundle][Validator] Remove remaining deprecations

| Q             | A
| ------------- | ---
| Branch?       | 7.0
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | -
| License       | MIT
| Doc PR        | symfony/symfony-docs#18781

Follow-up of #51425

Commits
-------

6e8cab7 [FrameworkBundle][Validator] Remove remaining deprecations
This was referenced Oct 21, 2023
@lyrixx
Copy link
Member

lyrixx commented Oct 25, 2023

see #52287

@alexandre-daubois
Copy link
Contributor Author

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

7 participants