Skip to content

[PropertyInfo] Document setting serializer_groups to null in the SerializerExtractor #13825

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
Oct 22, 2020

Conversation

GuilhemN
Copy link
Contributor

This PR fixes #13793 :)

If you'd like I can backport the part about serializer_groups begin mandatory in order to use the SerializerExtractor.


.. note::

The `serializer_groups` option must be provided in order to have a value different than `null` returned.
Copy link
Member

Choose a reason for hiding this comment

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

A minor thing. I know it's annoying, but RST format is a bit different than Markdown for code. All single backticks must be double backticks:

`serializer_groups` -> ``serializer_groups``

Copy link
Contributor Author

@GuilhemN GuilhemN Jun 13, 2020

Choose a reason for hiding this comment

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

Oh right, thanks :) I've been fooled by the single backtick for links 😅

@wouterj wouterj added this to the 5.2 milestone Oct 17, 2020
Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Hi!

I'm sorry for the slow response on this one. Can you please rebase this from master onto the 5.x branch and fix my 2 minor comments? Other than that, it's ready to be merged imho.


.. note::

The ``serializer_groups`` option must be provided in order to have a value different than ``null`` returned.
Copy link
Member

Choose a reason for hiding this comment

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

I would move this into a PHP comment in the above example:

- // List information.
+ // The `serializer_groups` option must be configured (may be set to null)

Comment on lines +448 to +445
If ``serializer_groups`` is set to ``null``, serializer groups metadata won't be checked but you will get only the properties
considered by the Serializer Component (notably the ``@Ignore`` annotation is taken into account).
Copy link
Member

Choose a reason for hiding this comment

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

I think it's best to remove the .. note:: and make this a normal paragraph

Suggested change
If ``serializer_groups`` is set to ``null``, serializer groups metadata won't be checked but you will get only the properties
considered by the Serializer Component (notably the ``@Ignore`` annotation is taken into account).
If ``serializer_groups`` is set to ``null``, serializer groups metadata won't be checked but you
will get only the properties considered by the Serializer Component (notably the ``@Ignore``
annotation is taken into account).

@javiereguiluz javiereguiluz changed the base branch from master to 5.x October 22, 2020 07:17
@javiereguiluz javiereguiluz merged commit 73695a2 into symfony:5.x Oct 22, 2020
@javiereguiluz
Copy link
Member

Thanks Guilhem! @wouterj I did the changes you proposed while merging.

@GuilhemN
Copy link
Contributor Author

GuilhemN commented Oct 22, 2020

My bad, I forgot to update my PR to take into account @wouterj suggestions after reading the notification...
Thank you Javier :)

@GuilhemN GuilhemN deleted the patch-2 branch October 22, 2020 08:06
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.

[PropertyInfo] Support using the SerializerExtractor with no group check
4 participants