zink-lang / zink

Rustic programming language that targets the Ethereum Virtual Machine
https://zink-lang.org
GNU General Public License v3.0
137 stars 12 forks source link

feat(zink-codegen): introduce derive macro for events #281

Open malik672 opened 3 days ago

malik672 commented 3 days ago

Resolves #255

clearloop commented 2 days ago

Hi @malik672, could you please follow conventional commits in your commit messages, for example for the title

# zink is the main package you modified
feat(zink-codegen): introduce derive macro for events 

and add details for what you modified in a specific commit message

# @4977bd5
-  feat(zink): generate ABIs for events
# @b23c12a
- feat(zink): introduce util function for converting type to string

that would be helpful for reviewing, thank you!

malik672 commented 2 days ago

Hi @malik672, could you please follow conventional commits in your commit messages, for example for the title

# zink is the main package you modified
feat(zink-codegen): introduce derive macro for events 

and add details for what you modified in a specific commit message

# @4977bd5
-  feat(zink): generate ABIs for events
# @b23c12a
- feat(zink): introduce util function for converting type to string

that would be helpful for reviewing, thank you!

done

clearloop commented 2 days ago

done

Oh sorry, I mean in your commit messages but not the description

Screenshot 2024-11-27 at 13 06 52

I'll help you edit the description, you just need to use the conventional commit messages in your future commits

NOTE

when you complete this PR, please refactor the log example and make sure the CI is green

https://github.com/zink-lang/zink/blob/main/examples/log.rs#L10

btw please add your polygon address in the description of this PR, I'll transfer the budget to you as soon as the PR merged!

malik672 commented 2 days ago

done

Oh sorry, I mean in your commit messages but not the description Screenshot 2024-11-27 at 13 06 52

I'll help you edit the description, you just need to use the conventional commit messages in your future commits

NOTE

when you complete this PR, please refactor the log example and make sure the CI is green

https://github.com/zink-lang/zink/blob/main/examples/log.rs#L10

btw please add your polygon address in the description of this PR, I'll transfer the budget to you as soon as the PR merged!

apart from the log refactor is anything still remaining for the PR ?

clearloop commented 2 days ago

apart from the log refactor is anything still remaining for the PR ?

The test is the main thing! because our purpose is introducing a nice developer interface ^ ^

#[derive(Event)]
pub enum ERC20Events {
   Transfer(from: Address, to: Address, value: U256),
   Approval(owner: Address, spender: Address, value: U256),
   // ...
}

#[zink::external]
fn foo(...) {
   ERC20Events::Transfer(from, to, value).emit()
}

If the described interface works, our PR works, we may encounter issues while implementing this interface, and they're the problems we need to solve in this PR, please pay attention to the inline document and the handbook as well since it will help others using our code!

malik672 commented 2 days ago

apart from the log refactor is anything still remaining for the PR ?

The test is the main thing! because our purpose is introducing a nice developer interface ^ ^

#[derive(Event)]
pub enum ERC20Events {
   Transfer(from: Address, to: Address, value: U256),
   Approval(owner: Address, spender: Address, value: U256),
   // ...
}

#[zink::external]
fn foo(...) {
   ERC20Events::Transfer(from, to, value).emit()
}

If the described interface works, our PR works, we may encounter issues while implementing this interface, and they're the problems we need to solve in this PR, please pay attention to the inline document and the handbook as well since it will help others using our code!

ohh ok, will update soon, can i solve the log issue here also ?

clearloop commented 2 days ago

ohh ok, will update soon, can i solve the log issue here also ?

do you have an issue number of the log issue you mentioned? some of the issues may be outdated after our new implementation, feel free to add more Resolves #xx in the description of the PR!

on the other hand, we have limited budget issues, it would be proper that not solving two budget issues in one PR ^ ^