Hey!

I think@mtarld has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@mtarld The original issue is marked asBug, should I target 4.x release or 6.1? I'd say it's rather a feature.

Also, I don't see any documentation for normalizers except forhttps://symfony.com/doc/5.2/serializer/normalizers.html#built-in-normalizers, I guess it's self-documented enough.

And lastly, I'm still wondering if the backward compatible behavior is needed in this edge case. Or at least what about adding a deprecation notice in case the original check does not pass so that we can delete it in a subsequent version? I mean I don't mind and most people configuring their normalizers can probably find this configuration option but it would prevent the unexpected first encounter of thisfeature 😄

Yes indeed, IMO it must be a feature 🙂

I guess the documentation, code, and context builders are enough.

For the BC migration path part, it is tricky. The behavior of these normalizers is changed (and the same goes for the serializer itself), so we must trigger a deprecation IMHO.

Maybe we could leverage thefunc_num_args function as it does not count the optional args that aren't explicitly given. This will hopefully work as theallowNormalizationOfObjectsWithoutXXX is the last property.
Something likeif (func_num_args() < 6) then trigger a deprecation that tells the user to explicitly give theallowNormalizationOfObjectsWithoutXXX as it'll default totrue in the 7.0 release.

WDYT?

Sounds good, I've added the deprecation, updated the default symfony bundle configuration (src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.php) and updated tests so that the deprecation is not triggered :)

Do we want to target 6.1 or 6.2 release?

As 6.1 is in feature freeze, we must target 6.2

Failing checks seem to be unrelated

Anything I can do to get a second review and merge it?