Fix docstring in EventContext #14145
Conversation
| If the event is a state event, this is normally the same as | ||
| ``state_group_before_event``. |
There was a problem hiding this comment.
Above, it says
state_group_before_event: ...
.... If
it's a state event, it will be the same as ``prev_group``.
so I think your interpretation is right, though I wonder why this says 'normally' the same?
I'd be tempted to suggest finding out why from someone who knows and documenting the circumstance in which it isn't, or given that the above documentation for state_group_before_event says that it will be the same, just striking out normally altogether.
There was a problem hiding this comment.
I can't think of any cases where they will be different.
There was a problem hiding this comment.
I'm a little bit hesitant to guarantee behaviour in docstrings like that, its not obvious to me that there is any reason why its useful to have a guarentee like that?
In particular, I've in the past toyed with the idea that prev_group for state events should not necessarily match that of state_group_before_event, and instead point to a state group of one of the events prev events. This might help with caches that have optimisations for when they've got a cached result for prev_group, in such cases they will almost never have a cached result for state_group_before_event when there has been a state resolution.
There was a problem hiding this comment.
Sounds like it makes sense to merge the change as is then?
Based on the other comments I think this is what was meant? Otherwise it doesn't make sense.