improve(events): add static emit function to Event macro#49
improve(events): add static emit function to Event macro#49scab24 wants to merge 3 commits intor55-eth:mainfrom
Conversation
- Add static emit() function to #[derive(Event)] macro - Simplify event emission syntax from log::emit(Event::new()) to Event::emit() - Update all event emissions in ERC20 contract - Update all event emissions in ERC721 contract - Add print_event_logs utility function for testing
0xrusowsky
left a comment
0xrusowsky
left a comment
There was a problem hiding this comment.
the macro change looks good, but style/lint should be fixed
examples/erc20/src/lib.rs
Outdated
|
|
||
| // Emit event + return | ||
| log::emit(Approval::new(owner, spender, amount)); | ||
| // Emit event + return usando la nueva sintaxis |
There was a problem hiding this comment.
remove spanish comments pls
There was a problem hiding this comment.
Deleted! I tried to automate the comments, but I forgot to review it
Thanks !
examples/erc20/src/lib.rs
Outdated
|
|
||
| // Emit event + return | ||
| log::emit(Transfer::new(from, to, amount)); | ||
| // Emit event + return usando la nueva sintaxis |
examples/erc20/src/lib.rs
Outdated
|
|
||
| // Emit event + return | ||
| log::emit(Transfer::new(from, to, amount)); | ||
| // Emit event + return usando la nueva sintaxis |
examples/erc20/src/lib.rs
Outdated
|
|
||
| // Emit event + return | ||
| log::emit(OwnershipTransferred::new(from, new_owner)); | ||
| // Emit event + return usando la nueva sintaxis |
r55/src/test_utils.rs
Outdated
| Bytes::from_hex(trimmed).expect("Unable to parse file content as bytes") | ||
| } | ||
|
|
||
| pub fn print_event_logs(logs: &[Log], title: Option<&str>) { |
There was a problem hiding this comment.
what's the point of the fn if we don't use it?
i'd either adopt it in the tests or remove it
There was a problem hiding this comment.
I used it to run some tests to verify that the new implementation was working correctly
I just removed it, thanks !
0xrusowsky
left a comment
0xrusowsky
left a comment
There was a problem hiding this comment.
lgtm, but u'll need an approval from @gakonst or @leonardoalt
|
Hi (@gakonst , @leonardoalt ), I would like to know if this PR is correct |
2 participants
#48