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

feat: create a way to handle errors #141

Closed
luislucena16 opened this issue Dec 14, 2023 · 8 comments · Fixed by #157
Closed

feat: create a way to handle errors #141

luislucena16 opened this issue Dec 14, 2023 · 8 comments · Fixed by #157
Assignees
Labels
good first issue Good for newcomers

Comments

@luislucena16
Copy link
Contributor

  • considering solidity updates and good semantic practices, it can be:
if (owner != msg.sender) revert NotAllowed();
  • helps to make error messages more verbose and simple to view code for developers or feedback to users.
@0xneves
Copy link
Contributor

0xneves commented Dec 14, 2023

Hi @luislucena16
Referring to the identation, I think its a good idea to shorten characters in the code by removing the "{}" of the if else's, from this:

        if (swap.owner != msg.sender) {
            revert InvalidAddress(msg.sender);
        }

...to this:

        if (swap.owner != msg.sender) revert InvalidAddress(msg.sender);

@0xneves 0xneves added the good first issue Good for newcomers label Dec 14, 2023
@0xneves 0xneves added this to Swaplace Dec 14, 2023
@0xneves 0xneves moved this to 🔖 TODO in Swaplace Dec 14, 2023
@luislucena16
Copy link
Contributor Author

hey @0xneves
yes exactly! it allows to handle errors more organized and with better semantics 💫

@0xjoaovpsantos
Copy link
Contributor

Hey guys this issue is available? If so, I would like to get it

@0xneves @luislucena16

@luislucena16
Copy link
Contributor Author

hi @0xjoaovpsantos I'll take care of it!

@0xneves 0xneves moved this from 🔖 TODO to 🛠️ In Progress in Swaplace Dec 19, 2023
@0xneves
Copy link
Contributor

0xneves commented Dec 19, 2023

Counting on you, this is a fast one, let's go!

@luislucena16
Copy link
Contributor Author

done! PR: #157

@0xneves
Copy link
Contributor

0xneves commented Dec 21, 2023

Could you speed up this resolution @luislucena16? Since it is blocking issue #104 from developing.
Thank you

@luislucena16
Copy link
Contributor Author

done! waiting for your approval

@0xneves 0xneves moved this from 🛠️ In Progress to ✅ Done in Swaplace Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants