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

[FVM] Refactor event emission code #4982

Merged
merged 3 commits into from
Nov 17, 2023

Conversation

janezpodhostnik
Copy link
Contributor

Just unified the event emission code a bit.

There is one point that requires special attention. The event size reported when you emit a cadence event is the payload size, however using the EmitFlowEvent method used the event type + payload size for the size.

I think the correct behaviour is the original one (just using the payload size) as the payload should contain the type as well.

Copy link
Contributor

@sideninja sideninja left a comment

Choose a reason for hiding this comment

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

Nice

@janezpodhostnik janezpodhostnik force-pushed the janez/change-event-emission-code branch from f466577 to e855f73 Compare November 15, 2023 15:35
@janezpodhostnik janezpodhostnik added this pull request to the merge queue Nov 15, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 15, 2023
@janezpodhostnik janezpodhostnik added this pull request to the merge queue Nov 15, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Nov 15, 2023
@codecov-commenter
Copy link

codecov-commenter commented Nov 16, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (ab5b93f) 56.30% compared to head (005ac82) 56.21%.

Files Patch % Lines
fvm/environment/event_emitter.go 52.38% 9 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4982      +/-   ##
==========================================
- Coverage   56.30%   56.21%   -0.10%     
==========================================
  Files         963      962       -1     
  Lines       90184    89783     -401     
==========================================
- Hits        50776    50468     -308     
+ Misses      35610    35549      -61     
+ Partials     3798     3766      -32     
Flag Coverage Δ
unittests 56.21% <54.54%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@janezpodhostnik janezpodhostnik added this pull request to the merge queue Nov 17, 2023
Merged via the queue into master with commit 66cb9b6 Nov 17, 2023
54 checks passed
@janezpodhostnik janezpodhostnik deleted the janez/change-event-emission-code branch November 17, 2023 12:00
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.

5 participants