Support Rails 8 #2483
Support Rails 8 #2483
Conversation
| @@ -74,7 +74,8 @@ def fragment_cache(cached_hash, non_cached_hash) | |||
| # see https://github.com/rails-api/active_model_serializers/pull/965 | |||
| # When <tt>options</tt> is +nil+, sets it to +{}+ | |||
| def serialization_options(options) | |||
| options ||= {} # rubocop:disable Lint/UselessAssignment | |||
| options ||= {} | |||
| options.frozen? ? options.dup : options | |||
There was a problem hiding this comment.
Curious if the cost of checking for freezing here is worth it vs. always duplicating.
There was a problem hiding this comment.
I ran a synthetic benchmark, and both options show almost identical performance:
user system total real
Always dup: 0.458805 0.015979 0.474784 ( 0.475676)
Cond. dup (frozen): 0.460826 0.014778 0.475604 ( 0.476971)
Cond. dup (not frozen): 0.459270 0.010303 0.469573 ( 0.471081)
For reference, here is the script I used.
|
@bf4 Thanks for your reply. The CI is already passing: https://github.com/pulkit110/active_model_serializers/actions/runs/11772425914 and there are no runtime differences: Please let me know if you need any changes from my side to get this ready for merge. |
pulkit110
force-pushed
the
0-10-stable
branch
from
4f31ebd to
da08a2c
Compare
|
FYI I have rebased my branch on 0-10-stable and the CI is passing: https://github.com/pulkit110/active_model_serializers/actions/runs/11866564182 |
|
Looks like this needs workflow approval after rebase? |
pulkit110
force-pushed
the
0-10-stable
branch
from
da08a2c to
f33e76c
Compare
|
I noticed that the workflow was failing, so I pushed a fix in f33e76c The workflow requires approval again. |
pulkit110
force-pushed
the
0-10-stable
branch
from
f33e76c to
b31d91c
Compare
|
Looks good to me. Let's merge this. |
|
Thanks for your work. Released 0.10.15 |
Purpose
Support Rails 8
Changes
There are two main changes:
dupit if it's frozen.ActiveSupport::Deprecation.silenceis replaced byRails.deprecator.silenceThis PR also adds Rails 7.2 and 8.0 to the test matrix.
Caveats
No caveats
Related GitHub issues
No related issues
Additional helpful information
This is my first contribution, so please let me know if something is missing and I'll try to answer it.