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

[test] #137 implement newVoteReceived event on Fund contract #148

Merged

Conversation

jorgezerpa
Copy link
Contributor

@jorgezerpa jorgezerpa commented Oct 24, 2024

Pull Request

Changes description

  • create an event NewVoteReceived in the Fund contract
  • Emit such event when call receiveVote function

Current output

image
image

Time spent breakdown

1 hour

Comments

I'm happy to contribute! I stay attentive to any feedback or needed modification

@EmmanuelAR EmmanuelAR self-requested a review October 24, 2024 22:49
contracts/src/fund.cairo Outdated Show resolved Hide resolved
@EmmanuelAR
Copy link
Collaborator

Hello @jorgezerpa thanks for the pr, so fast, there are some changes request. Thks again!

contracts/src/fund.cairo Outdated Show resolved Hide resolved
@jorgezerpa
Copy link
Contributor Author

Hello @EmmanuelAR I got it!🫡 I'll try to get it done by tomorrow before half of the day🚀

@EmmanuelAR
Copy link
Collaborator

Take your time Sr!

@jorgezerpa
Copy link
Contributor Author

jorgezerpa commented Oct 25, 2024

GM! changes ready:
image

Just 2 things before make the push:

  1. When I access to "self.up_votes.read()" should I add 1 on the event? (like "self.up_votes.read() + 1") or with the line "self.up_votes.write(self.up_votes.read() + 1);" that is above in the same function the value is already added when I try to read it again?
  2. Check the name of the keys, let me know if are OK or maybe you would like to change someone

@EmmanuelAR
Copy link
Collaborator

Hey @jorgezerpa

  1. For the votes just call the votes.read() because we already increment it.
  2. I cant check the name of the keys because you have to push the code
  3. please run scarb fmt

@jorgezerpa
Copy link
Contributor Author

@EmmanuelAR pushed!🙌

Copy link
Collaborator

@EmmanuelAR EmmanuelAR left a comment

Choose a reason for hiding this comment

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

Exc work @jorgezerpa LGTM!

@EmmanuelAR
Copy link
Collaborator

hey @jorgezerpa please merge your local branch with the latest version of the code, we add some events so the event trait is already create and you are adding twice fix like this example:

Captura de pantalla 2024-10-25 a la(s) 10 16 22 a  m

Also run scarb build and fmt

@jorgezerpa
Copy link
Contributor Author

jorgezerpa commented Oct 25, 2024

@EmmanuelAR Merged, fixed and pushed🫡

@EmmanuelAR EmmanuelAR merged commit 1ffa737 into undefinedorgcr:dev Oct 25, 2024
5 checks passed
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.

[feat] Emit event when fund receives a vote
2 participants