8000 [Typography] Fix SerializerAwareInterface description by idetox · Pull Request #38521 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Typography] Fix SerializerAwareInterface description #38521

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
Oct 12, 2020
Merged

[Typography] Fix SerializerAwareInterface description #38521

merged 1 commit into from
Oct 12, 2020

Conversation

idetox
Copy link
Contributor
@idetox idetox commented Oct 11, 2020 •
edited
Loading
Q A
Branch? 5.x
Bug fix? no
New feature? no
Deprecations? no
Tickets -
License MIT

Removed some unneeded comments in Serializer / Normalizer / Encoder directory

@idetox idetox requested a review from dunglas as a code owner October 11, 2020 17:13
Nyholm
Nyholm approved these changes Oct 11, 2020
View reviewed changes
Copy link
Member
@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Im happy with this.. or remove the comment all together.

derrabus
derrabus approved these changes Oct 11, 2020
View reviewed changes
Copy link
Member
@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

This patch should be applied to the 3.4 branch, though.

fabpot
fabpot reviewed Oct 12, 2020
View reviewed changes
src/Symfony/Component/Serializer/SerializerAwareInterface.php Outdated
@@ -12,7 +12,7 @@
namespace Symfony\Component\Serializer;

/**
* Defines the interface of encoders.
* Class accepting a serializer.
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 remove the comment instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fabpot would you remove the comment in NormalizerAwareInterface and DenormalizerAwareInterface aswell then ?

Copy link
Member

Choose a reason for hiding this comment

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

If they don't provide significant value, yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, expanded the removal to few other files

@xabbuh xabbuh added this to the 3.4 milestone Oct 12, 2020
@idetox idetox force-pushed the fix-serializer-serializerawareinterface-typography branch from 800310b to 32cf368 Compare October 12, 2020 12:18
Nyholm
Nyholm approved these changes Oct 12, 2020
View reviewed changes
Copy link
Member
@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Thank you

@fabpot fabpot changed the base branch from 5.x to 3.4 October 12, 2020 12:32
@fabpot
Copy link
Member
fabpot commented Oct 12, 2020

Thank you @idetox.

@fabpot fabpot force-pushed the fix-serializer-serializerawareinterface-typography branch from 32cf368 to 48e6976 Compare October 12, 2020 12:32
@fabpot fabpot merged commit f3e496f into symfony:3.4 Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@fabpot fabpot fabpot approved these changes

@Nyholm Nyholm Nyholm approved these changes

@derrabus derrabus derrabus approved these changes

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

@lyrixx lyrixx Awaiting requested review from lyrixx

@sroze sroze Awaiting requested review from sroze

@xabbuh xabbuh Awaiting requested review from xabbuh

Assignees
No one assigned
Labels
Serializer Status: Needs Review
Projects
None yet
Milestone
3.4
Development

Successfully merging this pull request may close these issues.
7 participants
@idetox @fabpot @nicolas-grekas @Nyholm @derrabus @xabbuh @carsonbot
0