Closed dandavison closed 8 months ago
One wonders if this makes more sense, and it can be validated as mutually exclusive with the singular one. I fear we're going to later regret not letting you choose the types to reapply.
Why use exclusion in the api instead of inclusion with a default to apply all?
I agree that a pure inclusion-based API would be preferable. I'll push a second commit along those lines shortly.
I agree that a pure inclusion-based API would be preferable. I'll push a second commit along those lines shortly.
We discussed this offline and concluded that exclusion semantics were best seeing as the default needs to be "all". I've included a commit implementing an inclusion variant, but reverted it.
These are proposed proto changes in preparation for Update reapply, based on a design by @yycptt.
Currently, the Reset request allows a user to send
RESET_REAPPLY_TYPE_SIGNAL
orRESET_REAPPLY_TYPE_NONE
. The server defaults toRESET_REAPPLY_TYPE_SIGNAL
.We are about to start reapplying Updates and, in the future, we may reapply other events such as Nexus-related events.
We do not want to expand the
ResetReapplyType
enum to encode arbitrary subsets of event types to be reapplied.Instead, this PR adds to the Reset request the ability to send a list of event types to be excluded from reapply.
I have also added one final value to the legacy enum:
RESET_REAPPLY_TYPE_ALL_ELIGIBLE
. I propose to make this the new default forResetReapplyType
. In practice, today, that has the same consequences (i.e. "reapply signals"), just under a different name.Other than that final addition,
ResetReapplyType
now becomes deprecated.Thus what I'm proposing is that, in the future, the server's implementation will be to:
ResetReapplyType
ResetReapplyExcludeType
eventsThe result is the set of event types that will be reapplied during Reset.