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

docs: cleanup wasm_hook.go #165

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

robert-zaremba
Copy link

Updated few comments and simplified the code.

Updated few comments and simplified the code.
@github-actions github-actions bot added the ibc-hooks Label for items related to Osmosis' ibc-hooks implementation label Feb 27, 2024
@@ -181,7 +177,6 @@ func ValidateAndParseMemo(memo string, receiver string) (isWasmRouted bool, cont
fmt.Errorf(types.ErrBadMetadataFormatMsg, memo, "wasm metadata is not a valid JSON map object")
}

// Get the contract
Copy link
Author

Choose a reason for hiding this comment

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

this comment doesn't bring anything. The implementation and the error message is clear and short, so it's better to read the code than the comment.
Rule 1: Comments should not duplicate the code

Copy link
Author

Choose a reason for hiding this comment

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

same comment applies to few changes below

@Reecepbcups
Copy link
Member

Reecepbcups commented May 22, 2024

The team just updated main to SDK v50, if you want to rebase on that I am happy to approve myself to get in the updated docs

If not I will update this PR when I can and get in

@Reecepbcups Reecepbcups changed the title chore: cleanup wasm_hook.go docs: cleanup wasm_hook.go May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ibc-hooks Label for items related to Osmosis' ibc-hooks implementation waiting-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants