Skip to content

[Serializer] Fixed docblocks and parameter names #33186

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

Merged
merged 1 commit into from
Aug 16, 2019

Conversation

derrabus
Copy link
Member

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #32179
License MIT
Doc PR N/A

Backports from #33185.

@@ -172,17 +172,17 @@ public function normalize($data, $format = null, array $context = [])
*
* @throws NotNormalizableValueException
*/
public function denormalize($data, $type, $format = null, array $context = [])
public function denormalize($data, $class, $format = null, array $context = [])
Copy link
Member Author

Choose a reason for hiding this comment

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

💡 The parameter is called $class in the interface. This change synchronizes the method signatures.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Aug 15, 2019
@dunglas
Copy link
Member

dunglas commented Aug 15, 2019

For the record, $class/$type can be other kind of types, including interfaces: https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/Serializer/Normalizer/DateTimeNormalizer.php#L31

However, as even interfaces define the constant ::class in PHP, using $class looks good enough for me.

@derrabus
Copy link
Member Author

derrabus commented Aug 15, 2019

@dunglas We can also change $class to $type everywhere. My concern was just to sync the parameters. In case of the ArrayDenoramalizer we accept SomeClass[], so $type might be more accurate.

@dunglas
Copy link
Member

dunglas commented Aug 15, 2019

Indeed, maybe should we use $type everywhere then.

@derrabus derrabus force-pushed the bugfix/serializer-types branch from 2aee03d to 50701fe Compare August 15, 2019 11:43
@derrabus
Copy link
Member Author

Done.

@nicolas-grekas
Copy link
Member

Thank you @derrabus.

@nicolas-grekas nicolas-grekas merged commit 50701fe into symfony:3.4 Aug 16, 2019
nicolas-grekas added a commit that referenced this pull request Aug 16, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

[Serializer] Fixed docblocks and parameter names

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32179
| License       | MIT
| Doc PR        | N/A

Backports from #33185.

Commits
-------

50701fe [Serializer] Fixed docblocks and parameter names.
@derrabus derrabus deleted the bugfix/serializer-types branch August 16, 2019 07:10
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.

5 participants