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

Support events outside of contracts #654

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

Maltby
Copy link
Contributor

@Maltby Maltby commented Feb 6, 2022

What was wrong?

Events can only be defined within contracts.
closes #80

How was it fixed?

Handle module level events in analyzer and lowering crates.

To-Do

  • OPTIONAL: Update Spec if applicable
  • Add entry to the release notes (may forgo for trivial changes)
  • Clean up commit history

@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2022

Codecov Report

Merging #654 (23d59a4) into master (897f159) will increase coverage by 0.06%.
The diff coverage is 90.32%.

❗ Current head 23d59a4 differs from pull request most recent head b65e3a4. Consider uploading reports for the commit b65e3a4 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #654      +/-   ##
==========================================
+ Coverage   85.81%   85.88%   +0.06%     
==========================================
  Files          97       98       +1     
  Lines        9075     9081       +6     
==========================================
+ Hits         7788     7799      +11     
+ Misses       1287     1282       -5     
Impacted Files Coverage Δ
crates/analyzer/src/namespace/items.rs 83.61% <25.00%> (+0.36%) ⬆️
crates/analyzer/src/db/queries/contracts.rs 92.38% <100.00%> (+0.03%) ⬆️
crates/analyzer/src/db/queries/module.rs 79.89% <100.00%> (+0.86%) ⬆️
crates/lowering/src/mappers/contracts.rs 100.00% <100.00%> (ø)
crates/lowering/src/mappers/events.rs 100.00% <100.00%> (ø)
crates/lowering/src/mappers/module.rs 84.33% <100.00%> (+1.20%) ⬆️
crates/parser/src/ast.rs 89.24% <0.00%> (+0.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 897f159...b65e3a4. Read the comment docs.

@Maltby Maltby marked this pull request as ready for review February 6, 2022 19:41
@Y-Nak
Copy link
Member

Y-Nak commented Feb 8, 2022

Thanks, @Maltby! And sorry for the late reply that causes some conflicts.
LGTM! I'll merge this PR when conflicts are resolved.

nit;

  • Could you add closes #{issue_number} to the PR comment in order to make the corresponding issue close automatically when the PR is merged?
  • Could you add 80.features.md in newsfragments? We use them for release notes.

@sbillig
Copy link
Collaborator

sbillig commented Feb 8, 2022

Sorry @Maltby, I should have merged this before I greedily merged my own changes that touch all sorts of analyzer code.

@Maltby
Copy link
Contributor Author

Maltby commented Feb 10, 2022

Thanks @Y-Nak ! I added closes #80 to the comment and created 80.features.md.

@sbillig No worries, I got it merged without much hassle :)

@Y-Nak
Copy link
Member

Y-Nak commented Feb 10, 2022

Thanks, @Maltby! Beautiful work!

@Y-Nak Y-Nak merged commit e917dc3 into ethereum:master Feb 10, 2022
@Maltby Maltby deleted the module-level-events branch February 11, 2022 01:20
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.

Support events outside of contracts
4 participants