Skip to content

[Serializer] Serialization versioning #35617

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

Closed

Conversation

ArnaudTarroux
Copy link

@ArnaudTarroux ArnaudTarroux commented Feb 6, 2020

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #30848
License MIT
Doc PR n/a

Add Serialization versioning.

  • Added @Since annotation
  • Added @Until annotation
  • Added support of since and until to the xml and yaml mapping
  • Added version context option
  • Added the possibility to configure the serializer.default_version in the framework config

@fabpot
Copy link
Member

fabpot commented Aug 18, 2020

@ArnaudTarroux Are you still motivated to work on this feature?

@ArnaudTarroux
Copy link
Author

@ArnaudTarroux Are you still motivated to work on this feature?

@fabpot Yes I am, I can work on it next week

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me. There is a pending question on the issue: how will one set the current version on real apps?

@ArnaudTarroux
Copy link
Author

Thanks, looks good to me. There is a pending question on the issue: how will one set the current version on real apps?

@nicolas-grekas In the serialization context with the key version

@nicolas-grekas
Copy link
Member

In the serialization context with the key version

do you think we need a config option to set the default version context?

@ArnaudTarroux ArnaudTarroux force-pushed the feature/serializer-version branch from 0ba9401 to 834c1d9 Compare August 31, 2020 19:56
@ArnaudTarroux
Copy link
Author

In the serialization context with the key version

do you think we need a config option to set the default version context?

Yes maybe in some cases it might be useful. I'll implement it 👍

@ArnaudTarroux
Copy link
Author

ArnaudTarroux commented Sep 1, 2020

Thanks, looks good to me. There is a pending question on the issue: how will one set the current version on real apps?

@nicolas-grekas thank for you review.
I've made some changes following your advices, including version_compare() instead of Composer/Semver.
And I've added the possibility to configure the default version in the framework config (serializer.default_version).

Copy link
Member

@yceruto yceruto left a comment

Choose a reason for hiding this comment

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

For consistency.

@fabpot fabpot closed this Oct 7, 2020
@fabpot
Copy link
Member

fabpot commented Oct 7, 2020

We've just moved away from master as the main branch to use 5.x instead. Unfortunately, I cannot reopen the PR and change the target branch to 5.x. Can you open a new PR referencing this one to not loose the discussion? Thank you for your understanding and for your help.

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.

6 participants