-
Notifications
You must be signed in to change notification settings - Fork 260
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
dependent events: followup #3760
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3760 +/- ##
==========================================
- Coverage 84.08% 84.02% -0.06%
==========================================
Files 260 260
Lines 27031 27036 +5
==========================================
- Hits 22730 22718 -12
- Misses 4301 4318 +17 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you; some questions.
@andybalaam For what it's worth, I've added a small commit that's a big "duh" from me: the previous PR introduced a transaction id for the dependent event, but forgot to make use of it. This isn't a big deal for edits/redacts, but it would have been a massive failure for reactions. I've addressed it here for edits/redacts, which isn't a big change, but wanted to let you know before I autosquash and merge later. |
All looks good to me, thanks! |
7ed0fd2
to
b023b7d
Compare
…istinguish the parent from the child transaction id
…network queries Intense facepalm energy here.
b023b7d
to
a466751
Compare
I'm tired. Here are some improvements/bugfixes for the dependent event mechanism.