Skip to content
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

RequestSerialization should be virtual #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FairlySadPanda
Copy link

RequestSerialization is technically a
built-in, but there's no reason to
prevent extension by developers
(or warn against it, depending on
the IDE).

An example of this usecase is
the common pattern of calling RequestSerialization() and then
OnDeserialization(). By extending
RequestSerialization we can declare
that we always will call
OnDeserialization afterwards, reducing
boilerplate.

Other examples: if we would like
to update an atomic clock by one
whenever we call RequestSerialization,
or we might want to automate assigning
ownership.

(clone of #34 with fixed source branch)

RequestSerialization is technically a
built-in, but there's no reason to 
prevent extension by developers
(or warn against it, depending on
the IDE).

An example of this usecase is
the common pattern of calling `RequestSerialization()` and then
`OnDeserialization()`. By extending
`RequestSerialization` we can declare
that we always will call 
`OnDeserialization` afterwards, reducing
boilerplate.

Other examples: if we would like
to update an atomic clock by one
whenever we call RequestSerialization,
or we might want to automate assigning
ownership.
@FairlySadPanda FairlySadPanda force-pushed the FairlySadPanda/RequestSerialization-Should-Be-Virtual branch from 0952f8c to d22a42c Compare May 31, 2022 17:51
@FairlySadPanda
Copy link
Author

FairlySadPanda commented May 31, 2022

Apologies for the weirdness, my fork wasn't set up correctly. This has been fixed and my rebase workflow enacted so I should be able to keep this PR and my others nice and clean. You lot will probably want to kick CLA Assistant as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant