Issue #283: Add support for custom serialization groups#287
Issue #283: Add support for custom serialization groups#287kiler129 wants to merge 1 commit intoalgolia:masterfrom
Conversation
kiler129
force-pushed
the
fix-#283
branch
2 times, most recently
from
0566a27 to
2452885
Compare
|
@nunomaduro Is there a chance of merging this FR? (trying to decide whatever to wait or maintain a private fork for now ;)) |
|
Not this Friday.
I will take a look next week.
…On Fri 15 Feb 2019 at 17:41, Grzegorz Zdanowski ***@***.***> wrote:
@nunomaduro <https://github.com/nunomaduro> Is there a chance of merging
this FR? (trying to decide whatever to wait or maintain a private fork for
now ;))
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#287 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFNFVKdsdYaEluopmXrMfTqTequTuI-Xks5vNuNBgaJpZM4a4son>
.
|
|
Fridays MUST be read-only :) |
nunomaduro
left a comment
nunomaduro
left a comment
There was a problem hiding this comment.
I need to review this more, were are the comments so far.
| (!isset($v['enable_serializer_groups']) || !$v['enable_serializer_groups']); | ||
| }) | ||
| ->thenInvalid('In order to specify "serializer_groups" you need to enable "enable_serializer_groups"') | ||
| ->end() |
There was a problem hiding this comment.
This shouldn't have ->defaultFalse()?
There was a problem hiding this comment.
I don't see a reason why this node should (or can it even?) get a default value of false?
This is just to prevent putting custom groups and wondering why this doesn't work (so really an DX improvement here).
Am I missing something?
|
@nunomaduro I addressed the comments - let me know if anything else should be changed. |
|
@nunomaduro Can I do anything to make this issue mergable? |
|
@nunomaduro Any progress on this review? |
|
@kiler129 This feature/pr it's in our todolist. Unfortunately we didn't found time yet for the review. I am sorry. |
|
Hey @kiler129 , Thanks again for your contribution! Unfortunately we still don't have the time to review it so I'm putting it on hold for the moment. Thanks for your patience! |
|
PR looks great, unfortunately we use JMS so no use for us. This will definitely need to have conflicts solved before maintainers can have a look here though. |
4 participants
Describe your change
rootEntitycontaining FQCN of the entity defined for the indexserializer_groupsallows specifying serialization groups to use on per index basisWhat problem is this fixing?
See #283 for detailed description.
Background
In essence this came up with our enterprise integration - I found a fix so I decided to poke around in my free time (so no worry here for copyright, patch is MIT).