Skip to content

Conversation

@jsharpify
Copy link
Contributor

Instead of reprocessing every serializer everytime a new serializer is added, just process the new one. This has the intended side-effect of only issuing the deprecation warning for non-compliant serializers once (when added) instead of on every subsequent call to add serializers. This also ensures that the location deperecation warning does not point to the innocent compliant serializers. Currently when it reports the deprecation it will include the location of the most recent call to add_serializers which falsely implicates those serializers.

skipkayhil
skipkayhil previously approved these changes Oct 23, 2025
@rafaelfranca
Copy link
Member

CI is broken

@jsharpify jsharpify force-pushed the jsharpify-reduce-deprecation-warnings branch from 2702429 to 6e956aa Compare October 23, 2025 20:10
Comment on lines 55 to 56
@serializers_index.clear
add_new_serializers(serializers)
Copy link
Member

Choose a reason for hiding this comment

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

@serializers is a Set, but aren't we adding them twice now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, yup. Emptying it out first and then assigning

@jsharpify jsharpify force-pushed the jsharpify-reduce-deprecation-warnings branch from 6e956aa to a292f53 Compare October 23, 2025 23:29
Instead of reprocessing every serializer everytime a new serializer is added, just process the new one.
This has the intended side-effect of only issuing the deprecation warning for non-compliant serializers
once (when added) instead of on every subsequent call to add serializers. This also ensures that the
filepath in the deperecation warning does not point to the innocent compliant serializers.
@jsharpify jsharpify force-pushed the jsharpify-reduce-deprecation-warnings branch from a292f53 to 9005422 Compare October 23, 2025 23:42
@rafaelfranca rafaelfranca merged commit f73b7f1 into rails:main Oct 24, 2025
4 checks passed
rafaelfranca added a commit that referenced this pull request Oct 24, 2025
…-warnings

ActiveJob/serializers: Only index new serializers
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.

3 participants