ActiveJob: delegate deserialization to the job class #18260
Conversation
|
cc @dhh |
|
Looks good, but I don't think we need to change the class method. It's fine to have the class and instance methods both be called deserialize. |
|
Updated. ActiveJob specs still pass locally. Shall I squash? Also, is there any chance of this making it onto 4-2-stable? I'm pretty sure it's completely non-breaking. |
|
Please squash. I don't think it's a good idea to apply to stable. It's a feature upgrade.
|
b3fb0ec to
96e196e
Compare
|
Cool, squashed 💃 |
|
Just need some docs too. Explain why the change was needed. Maybe give an example of how you'd overwrite the serializers. |
96e196e to
1a9ddad
Compare
|
Added docs and an example (it's pretty in-depth but I can't think of another decent one - shout if you want me to it down). Not sure where I should explain the change - the ActiveJob CHANGELOG is empty and I can't find another AJ PR with a CHANGELOG entry. |
|
You can have the honors of starting the CHANGELOG then :D |
|
Just follow the pattern for any of the other frameworks. |
1a9ddad to
65542e2
Compare
|
Done 🎉 |
|
Lovely! |
…ialize ActiveJob: delegate deserialization to the job class
Indent the list content by 4 spaces instead of 2 to match the other changelog files. Also wrap the lines around 80 chars. Finally update the documentation example with nit-picky things.
Renames
ActiveJob::Base.deserializetoActiveJob::Base.instantitate_from_job_data(not convinced this is a good name), and delegates full deserialization toActiveJob::Base#deserialize. This allows subclasses to overrideActiveJob::Base#deserialize(they can already override#serialize) to attach metadata to the job. Without this, the only way to attach metadata to a job and retrieve it is by monkey-patchingActiveJob::Base.deserialize🙈