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

Move serializer to form directory #737

Merged
merged 1 commit into from
Apr 15, 2020
Merged

Move serializer to form directory #737

merged 1 commit into from
Apr 15, 2020

Conversation

wbloszyk
Copy link
Member

@wbloszyk wbloszyk commented Apr 9, 2020

Subject

This PR move serializer to form dictionary, becouse serializer is always used with BaseDoctrineORMSerializationType.

I am targeting this branch, because this is PATCH.

Closes #733

Changelog

### Added
- Added `Sonata\Form\Serializer\BaseSerializerHandler`
- Added `Sonata\Form\Serializer\BaseSerializerHandlerInterface` 
### Deprecated
- Deprecated `Sonata\Serializer\BaseSerializerHandler`
- Deprecated `Sonata\Serializer\BaseSerializerHandlerInterface`

@wbloszyk
Copy link
Member Author

wbloszyk commented Apr 9, 2020 •
edited
Loading

RTM, but I add notice deprecation to 3.19.0. Should I change it to 3.18.1 or you will release it as minor?

@greg0ire
Copy link
Contributor

greg0ire commented Apr 9, 2020

Change it to 3.x, I usually change the numbers when I release.

@greg0ire greg0ire added the patch label Apr 9, 2020
@greg0ire
Copy link
Contributor

greg0ire commented Apr 9, 2020

This occurrence should be updated to use the new name:

SonataCoreBundle/src/CoreBundle/DependencyInjection/SonataCoreExtension.php

Line 201 in d0f376d

BaseSerializerHandler::setFormats($config['serializer']['formats']);

@wbloszyk
Copy link
Member Author

wbloszyk commented Apr 9, 2020

Change it to 3.x, I usually change the numbers when I release.

OK. I will remember about it. Should I change it here?

This occurrence should be updated to use the new name:

SonataCoreBundle/src/CoreBundle/DependencyInjection/SonataCoreExtension.php

Line 201 in d0f376d

BaseSerializerHandler::setFormats($config['serializer']['formats']);

Sorry for that. So many repeated.

BTW. After release serializer PATCH, I can add it to 0.x extensions backport. Then Core4 + backport extensions will be ready to review. (I test it from my repositories and it is working).

@greg0ire
Copy link
Contributor

greg0ire commented Apr 9, 2020

OK. I will remember about it. Should I change it here?

No need, keep it that way.

@wbloszyk
Copy link
Member Author

@greg0ire It is RTM. Can you check it?

greg0ire
greg0ire previously approved these changes Apr 11, 2020
View reviewed changes
phansys
phansys suggested changes Apr 11, 2020
View reviewed changes
src/Form/Serializer/BaseSerializerHandler.php Outdated Show resolved Hide resolved
src/Serializer/BaseSerializerHandler.php Outdated Show resolved Hide resolved
src/Serializer/SerializerHandlerInterface.php Outdated Show resolved Hide resolved
phansys
phansys suggested changes Apr 11, 2020
View reviewed changes
Copy link
Member

@phansys phansys left a comment

Choose a reason for hiding this comment

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

Regarding the deprecation messages, I wonder if it's right to refer version 4.0, since (this bundle is deprecated and) it isn't intended to be released.

src/CoreBundle/Serializer/BaseSerializerHandler.php Outdated Show resolved Hide resolved
src/CoreBundle/Serializer/SerializerHandlerInterface.php Outdated Show resolved Hide resolved
@wbloszyk
Copy link
Member Author

It is little confusing but it is OK.

phansys
phansys previously approved these changes Apr 11, 2020
View reviewed changes
@wbloszyk
Copy link
Member Author

@greg0ire It is RTM and checked.

@greg0ire
Copy link
Contributor

Both Travis and FlintCI are failing, please run make cs-fix

@wbloszyk
Copy link
Member Author

Travis is failing becouse FlintCl. FlintCl is failing but this PR is not the reason. Look at it.

greg0ire
greg0ire suggested changes Apr 13, 2020
View reviewed changes
Copy link
Contributor

@greg0ire greg0ire left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Well then it means the 3.x build is currently failing, and this PR is blocked until it is fixed.

EDIT: blocked by #738

@wbloszyk @phansys @greg0ire
Move serializer to form
85beaea 
Update src/Form/Serializer/BaseSerializerHandler.php

Co-Authored-By: Javier Spagnoletti <phansys@gmail.com>

Update src/Serializer/SerializerHandlerInterface.php

Co-Authored-By: Javier Spagnoletti <phansys@gmail.com>

Update src/Serializer/BaseSerializerHandler.php

Co-Authored-By: Javier Spagnoletti <phansys@gmail.com>

Update src/Form/Serializer/BaseSerializerHandler.php

Co-Authored-By: Javier Spagnoletti <phansys@gmail.com>

Update src/CoreBundle/Serializer/BaseSerializerHandler.php

Co-Authored-By: Javier Spagnoletti <phansys@gmail.com>

Update src/CoreBundle/Serializer/SerializerHandlerInterface.php

Co-Authored-By: Javier Spagnoletti <phansys@gmail.com>
@wbloszyk
Copy link
Member Author

@phansys @greg0ire Can you merge it? I need it before start working on #739

@greg0ire greg0ire merged commit 65a57fe into sonata-project:3.x Apr 15, 2020
@greg0ire
Copy link
Contributor

Sure! Thanks @wbloszyk !

@wbloszyk wbloszyk deleted the move_serializer branch April 15, 2020 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@greg0ire greg0ire greg0ire approved these changes

@core23 core23 core23 approved these changes

@phansys phansys phansys left review comments

Assignees
No one assigned
Labels
patch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

What should we do with the serializer namespace?
4 participants
@wbloszyk @greg0ire @phansys @core23