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

Adds more descriptive and helpful error messages to the FT contracts and txs #170

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

joshuahannan
Copy link
Member

@joshuahannan joshuahannan commented Sep 10, 2024

Closes: #169

Based on guidelines in onflow/docs#795

Description

  • Updates all error messages to include more information and relevant data about the error if possible
  • Updates CLI commands for C1.0

For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Re-reviewed Files changed in the Github PR explorer

Copy link
Contributor

@sisyphusSmiling sisyphusSmiling left a comment

Choose a reason for hiding this comment

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

Very nice error messages. Couple minor comments and formatting suggestions.

contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
transactions/generic_transfer_with_paths.cdc Outdated Show resolved Hide resolved
transactions/safe_generic_transfer.cdc Outdated Show resolved Hide resolved
transactions/switchboard/transfer_tokens.cdc Outdated Show resolved Hide resolved
transactions/transfer_many_accounts.cdc Outdated Show resolved Hide resolved
transactions/transfer_tokens.cdc Outdated Show resolved Hide resolved
Copy link
Member

@nvdtf nvdtf left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! I left minor edits about punctuations and spacing

contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
contracts/FungibleToken.cdc Outdated Show resolved Hide resolved
transactions/switchboard/transfer_tokens.cdc Outdated Show resolved Hide resolved
transactions/transfer_many_accounts.cdc Outdated Show resolved Hide resolved
transactions/transfer_many_accounts.cdc Outdated Show resolved Hide resolved
transactions/transfer_tokens.cdc Outdated Show resolved Hide resolved
transactions/transfer_tokens.cdc Outdated Show resolved Hide resolved
Copy link
Member Author

@joshuahannan joshuahannan left a comment

Choose a reason for hiding this comment

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

Thank you for all the great comments! I addressed all of them

@joshuahannan joshuahannan merged commit 65c59eb into master Sep 24, 2024
2 checks passed
@joshuahannan joshuahannan deleted the error-msgs branch September 24, 2024 15:49
@ksvnar
Copy link

ksvnar commented Sep 28, 2024

Спасибо вам очень хорошо

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.

Update FT Error messages to be more descriptive
4 participants