Skip to content
This repository has been archived by the owner on Nov 5, 2021. It is now read-only.

Update jms/serializer-uuid-bundle #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

p4veI
Copy link

@p4veI p4veI commented Oct 17, 2021

Updates compatibility to support jms/serializer-bundle:^4.0

I'm running into symfony update issues while using the bundle. Seems it would require bumping the jms/serializer requirements in the serializer-uuid library too. (here mhujer/jms-serializer-uuid#13)

@enumag
Copy link

enumag commented Oct 29, 2021

ping @mhujer

@mhujer
Copy link
Owner

mhujer commented Oct 30, 2021

Hi @p4veI @enumag I was thinking about deprecating this bundle, because all it does is that it registeres a handler in DI. See https://github.com/mhujer/jms-serializer-uuid-bundle/blob/master/src/DependencyInjection/config/services.yml

The whole bundle can be replaced with this in services.yaml:

services:
    Mhujer\JmsSerializer\Uuid\UuidSerializerHandler:
        tags:
            - { name: jms_serializer.subscribing_handler }

I already did similar thing in my fork of consistence-jms-serializer-symfony - just stopped using the bundle and I register the handler manually.

What do you think? Registering the handler directly should allow easier updates of JMS library without being blocked by neither of mhujer/jms-serializer-uuid or mhujer/jms-serializer-uuid-bundle.

@enumag
Copy link

enumag commented Oct 30, 2021

Agreed, just add it into the readme of https://github.com/mhujer/jms-serializer-uuid. :-)

@mhujer
Copy link
Owner

mhujer commented Oct 30, 2021

@enumag I will do it, just wanted to check with you that there isn't any other reason why the bundle may be useful.

@p4veI
Copy link
Author

p4veI commented Oct 31, 2021

Thanks for the info, I've replaced the library anyways, just bumped the deps for others, but you're right using this as a bundle is just an extra overhead. Thanks for your work tho!

mhujer added a commit to mhujer/jms-serializer-uuid that referenced this pull request Nov 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants