Skip to content
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

Allow interfaces for typed properties #1254

Merged
merged 1 commit into from
Nov 8, 2020
Merged

Allow interfaces for typed properties #1254

merged 1 commit into from
Nov 8, 2020

Conversation

marein
Copy link
Contributor

@marein marein commented Oct 12, 2020

Q A
Bug fix? no
New feature? yes
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #...
License MIT

Currently the TypedPropertiesDriver only reads classes (abstract included) and some php defined types. For interfaces, additional metadata in the form of annotations or files must be specified. This pull request makes this obsolete.

@@ -1356,6 +1356,10 @@ public function testTypedProperties()
$this->markTestSkipped(sprintf('%s requires PHP 7.4', __METHOD__));
}

$builder = SerializerBuilder::create($this->handlerRegistry, $this->dispatcher);

Choose a reason for hiding this comment

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

@marein Hi,
nice idea, and thanks for pull-request ! 👍

Are there any things to consider during Deserialization process, no changes there?

Would be possible to have a Deserialization test, maybe ? :)

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @slava-v, thank you. The deserialization process is covered here. Did I miss something?

@dgafka
Copy link
Contributor

dgafka commented Nov 4, 2020

Really nice, thank you for feature :). This and #1256 I will really find useful

@goetas is this something that can be merged? :)

@goetas
Copy link
Collaborator

goetas commented Nov 8, 2020

seems good! thanks!

@goetas goetas merged commit 0b740bc into schmittjoh:master Nov 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants