8000 [Serializer] Remove wrong final tags by mtarld · Pull Request #52646 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Serializer] Remove wrong final tags #52646

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 20, 2023
Merged

[Serializer] Remove wrong final tags #52646

merged 1 commit into from
Nov 20, 2023

Conversation

mtarld
Copy link
Contributor
@mtarld mtarld commented Nov 20, 2023 •
edited
Loading
Q A
Branch? 6.4
Bug fix? no
New feature? no
Deprecations? no
Issues
License MIT

When @final was added to Groups and Context, the related and therefore deprecated tests weren't removed.
This PR removes them.

According to #52646 (comment), final should not have been added to attributes in the first time, this PR removes them.

@mtarld mtarld requested a review from dunglas as a code owner November 20, 2023 07:45
@carsonbot carsonbot added this to the 6.4 milestone Nov 20, 2023
@mtarld mtarld mentioned this pull request Nov 20, 2023
Merged
@nicolas-grekas
Copy link
Member

Apparently we made a mistake : the serializer attributes should not be final. We test them using is_a, which accounts for child classes:

symfony/src/Symfony/Component/Serializer/Mapping/Loader/AttributeLoader.php

Lines 254 to 263 in 80db691

private function isKnownAttribute(string $attributeName): bool
{
foreach (self::KNOWN_ATTRIBUTES as $knownAttribute) {
if (is_a($attributeName, $knownAttribute, true)) {
return true;
}
}
return false;
}

Can you please update the PR accordingly?

@mtarld mtarld force-pushed the fix/remove-deprecated-final-related-tests branch from aade72a to 74eeada Compare November 20, 2023 08:09
@mtarld mtarld changed the title [Serializer] Remove deprecated final related tests [Serializer] Remove wrong final tags Nov 20, 2023
@nicolas-grekas
Copy link
Member

Thank you @mtarld.

@nicolas-grekas nicolas-grekas merged commit 19c0d9a into symfony:6.4 Nov 20, 2023
@mtarld mtarld deleted the fix/remove-deprecated-final-related-tests branch November 20, 2023 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xabbuh xabbuh xabbuh approved these changes

@dunglas dunglas Awaiting requested review from dunglas dunglas is a code owner

Assignees
No one assigned
Labels
Serializer Status: Reviewed
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.
4 participants
@mtarld @nicolas-grekas @xabbuh @carsonbot
0