Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Improve documentation for EventContext fields #6319

Merged
merged 4 commits into from
Nov 5, 2019
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Nov 4, 2019

No description provided.

@richvdh richvdh requested a review from a team November 4, 2019 17:04
@richvdh richvdh force-pushed the rav/event_context/1 branch 2 times, most recently from 96f4455 to b70e641 Compare November 4, 2019 17:18
@richvdh richvdh force-pushed the rav/event_context/1 branch from b70e641 to 069098d Compare November 4, 2019 18:11
For a *rejected* state event, where the state of the rejected event is
ignored, this state_group should never make it into the
event_to_state_groups table. Indeed, inspecting this value for a rejected
state event is almost certainly incorrect.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I wonder if we should make accessing state_group raise if its rejected)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup could do!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well who knew, this identified a bug. I'm going to pull this out to a separate PR.

For an outlier, where we don't have the state at the event, this will be
None.

prev_group: If it is known, ``state_group``'s prev_group. Note that this being
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth noting that this is not the state group associated with prev_state_ids

Per review comments: add some sanity checks on accessing state_group etc for
rejected events.
This reverts commit f84119b.

This turned out to identify some bugs, so I'm going to pull the whole lot out
to a separate PR.
@richvdh richvdh merged commit 4086002 into develop Nov 5, 2019
@richvdh richvdh deleted the rav/event_context/1 branch November 5, 2019 13:24
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit '408600282':
  Improve documentation for EventContext fields (#6319)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants