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.

Sign up for GitHub

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

Jump to bottom

refactor: rename globalEvents to global_events #191

Merged
merged 1 commit into from
Jan 22, 2023
Merged

refactor: rename globalEvents to global_events #191

merged 1 commit into from
Jan 22, 2023

Conversation

Sekhmet
Copy link
Contributor

@Sekhmet Sekhmet commented Jan 19, 2023

Summary

Renaming globalEvents to global_events to match other config options.

@Sekhmet Sekhmet requested a review from bonustrack January 19, 2023 15:49
@Sekhmet Sekhmet self-assigned this Jan 19, 2023
@bonustrack
Copy link
Contributor

How do you define ABI for global events?

@Sekhmet
Copy link
Contributor Author

Sekhmet commented Jan 19, 2023

@bonustrack it's not possible, I'm not sure if it should be possible - event names are generic so there might not be a single ABI for event named XYZ, so I imagine people working with those might want to handle those themselves, check if event is valid etc. but maybe it makes sense to still provide option to define ABI per event.

@bonustrack
Copy link
Contributor

bonustrack commented Jan 20, 2023 •
edited
Loading

Right, let's keep it without ABI for now. Do we return the event in rawEvent with empty / undefined event?

@Sekhmet
Copy link
Contributor Author

Sekhmet commented Jan 20, 2023

Yes: https://github.com/snapshot-labs/checkpoint/blob/e61e967b5cf4590448c0b991cc364b0a87881477/src/providers/starknet/index.ts#L96

bonustrack
bonustrack approved these changes Jan 20, 2023
View reviewed changes
Copy link
Contributor

@bonustrack bonustrack left a comment

Choose a reason for hiding this comment

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

utACK

@Sekhmet Sekhmet merged commit 392c5a3 into master Jan 22, 2023
@Sekhmet Sekhmet deleted the sekhmet/global-events branch January 22, 2023 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bonustrack bonustrack bonustrack approved these changes

Assignees

@Sekhmet Sekhmet

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Sekhmet @bonustrack