-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[ObjectMapper] dump mapping code #61515
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
base: 7.4
Are you sure you want to change the base?
Conversation
bfb293c
to
af4be68
Compare
#61497 has been merged now. |
@soyuka maybe you can leverage the Filesystem component like it has been done in the JsonStreamer? |
src/Symfony/Component/ObjectMapper/Tests/Fixtures/SimpleSource.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/ObjectMapper/Tests/Fixtures/SimpleTarget.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some comments.
I'd be great to have a few fixtures of generated files.
Next steps are cache warmups (#61528 could be used for inspiration)
and FrameworkBundle integration of course.
Up to continue that way?
8d7e35c
to
3d52bc3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, here are some minor stuff, LGTM after that!
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Resources/config/object_mapper.php
Outdated
Show resolved
Hide resolved
...Symfony/Component/ObjectMapper/Tests/Fixtures/generated/a460713f6773437414a547074965e45e.php
Outdated
Show resolved
Hide resolved
...Symfony/Component/ObjectMapper/Tests/Fixtures/generated/a460713f6773437414a547074965e45e.php
Outdated
Show resolved
Hide resolved
return null; | ||
} | ||
|
||
public static function call(callable $fn, mixed $value, object $source, ?object $target = null): mixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need this helper? why not doing this instead? what's special about string callables?
$fn($value, $source, $target)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because we'll give $source
and $target
to a function that probably do not handle that (ucfirst
for example). Maybe it was not a good idea to support native php functions afterall and we should only supports callables?
91fd236
to
8f46a4f
Compare
@nicolas-grekas I extracted the part dumping code to another class in |
fb21219
to
8bf4b10
Compare
needs #61497
Benchmark code