8000 feat(events): add events by Rubilmax · Pull Request #107 · morpho-org/morpho-blue · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(events): add events #107

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
Merged
merged 22 commits into from
Aug 3, 2023
Merged

feat(events): add events #107

merged 22 commits into from
Aug 3, 2023

Conversation

Rubilmax
Copy link
Collaborator
@Rubilmax Rubilmax commented Jul 11, 2023 •
edited
Loading

This was linked to issues Jul 11, 2023
Classic Ownable(or Owned) or Ownable2Step #15
Closed
Add events #97
Closed
@Rubilmax Rubilmax changed the base branch from refactor/storage-accesses to main July 12, 2023 07:26
@Rubilmax Rubilmax changed the base branch from main to feat/approval-with-sig July 24, 2023 09:42
@Rubilmax Rubilmax removed a link to an issue Jul 24, 2023
Classic Ownable(or Owned) or Ownable2Step #15
Closed
@Rubilmax Rubilmax self-assigned this Jul 31, 2023
Base automatically changed from feat/approval-with-sig to main July 31, 2023 21:10
@MerlinEgalite MerlinEgalite marked this pull request as ready for review August 1, 2023 14:07
makcandrov
makcandrov reviewed Aug 1, 2023
View reviewed changes
Rubilmax
Rubilmax commented Aug 1, 2023
View reviewed changes
MathisGD
MathisGD reviewed Aug 1, 2023
View reviewed changes
MerlinEgalite
MerlinEgalite requested changes Aug 1, 2023
View reviewed changes
@MerlinEgalite MerlinEgalite requested a review from MathisGD August 2, 2023 17:08
@pakim249CAL
Copy link
Contributor
pakim249CAL commented Aug 2, 2023 •
edited
Loading

Sorry I think this idea came a bit late, but now that's there qualified access to events in other contracts starting in 0.8.21, maybe we can just put these events into IBlue? Not blocking. Can be done in a future PR.

pakim249CAL
pakim249CAL previously approved these changes Aug 2, 2023
View reviewed changes
MathisGD
MathisGD reviewed Aug 2, 2023
View reviewed changes
@MathisGD MathisGD mentioned this pull request Aug 2, 2023
Closed
MathisGD
MathisGD reviewed Aug 2, 2023
View reviewed changes
Rubilmax
Rubilmax commented Aug 3, 2023
View reviewed changes
Copy link
Collaborator Author
@Rubilmax Rubilmax left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This works (so I approve), but I'll never be convinced that not internalizing a storage update + its corresponding event is a good practice, especially when it's done twice in the code

@MerlinEgalite
Copy link
Contributor

This works (so I approve), but I'll never be convinced that not internalizing a storage update + its corresponding event is a good practice, especially when it's done twice in the code

Agree

makcandrov
makcandrov reviewed Aug 3, 2023
View reviewed changes
MathisGD
MathisGD reviewed Aug 3, 2023
View reviewed changes
@makcandrov makcandrov mentioned this pull request Aug 3, 2023
Merged
Rubilmax
Rubilmax commented Aug 3, 2023
View reviewed changes
Copy link
Collaborator Author
@Rubilmax Rubilmax left a comment

Choose a reason for hiding this comment

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

LGTM

@MathisGD MathisGD merged commit 66d0bf3 into main Aug 3, 2023
@MathisGD MathisGD deleted the feat/events branch August 3, 2023 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MerlinEgalite MerlinEgalite MerlinEgalite approved these changes

@julien-devatom julien-devatom julien-devatom approved these changes

@MathisGD MathisGD MathisGD approved these changes

@Jean-Grimal Jean-Grimal Jean-Grimal approved these changes

@makcandrov makcandrov makcandrov approved these changes

@QGarchery QGarchery Awaiting requested review from QGarchery

@ghivert ghivert Awaiting requested review from ghivert

@oumar-fall oumar-fall Awaiting requested review from oumar-fall

@mlgarchery mlgarchery Awaiting requested review from mlgarchery

@tomrpl tomrpl Awaiting requested review from tomrpl

@peyha peyha Awaiting requested review from peyha

@pakim249CAL pakim249CAL Awaiting requested review from pakim249CAL

Assignees

@MerlinEgalite MerlinEgalite

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add events
8 participants
@Rubilmax @MathisGD @MerlinEgalite @julien-devatom @pakim249CAL @peyha @Jean-Grimal @makcandrov
0