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

merge stargate and cosmwasm-stargate into finschia #37

Merged
merged 15 commits into from
Sep 22, 2022
Merged

Conversation

loin3
Copy link
Contributor

@loin3 loin3 commented Sep 8, 2022

#36

add finschia package that contains all the feature that lbm-sdk supports.

add additional github actions yml file for testing finschia. finschia has same tests with cosmwasm-stargate and stargate, running tests such as querying balances in same simapp fails. so I separate github actions into running finschia tests and other packages tests.

@loin3 loin3 linked an issue Sep 8, 2022 that may be closed by this pull request
@loin3 loin3 marked this pull request as draft September 8, 2022 01:56
@loin3 loin3 self-assigned this Sep 8, 2022
@loin3 loin3 marked this pull request as ready for review September 8, 2022 08:31
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

MsgVoteWeighted is added in in our gov module. So the messages.ts and aminomessages.ts of gov module is need.

"description": "Utilities for LBM SDK 0.45.0-rc7",
"contributors": [
"Simon Warta <webmaster128@users.noreply.github.com>",
"zemyblue <zemyblue@users.noreply.github.com>"
Copy link
Member

Choose a reason for hiding this comment

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

How about adding @loin3 's email below this line?

packages/finschia/src/utils.ts Show resolved Hide resolved
Copy link
Contributor Author

@loin3 loin3 left a comment

Choose a reason for hiding this comment

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

I did not take gov module because I found that latest cosmjs has same MsgVoteWighted . So if I bump up later, it will be the same.

@loin3 loin3 requested a review from zemyblue September 16, 2022 09:53
@zemyblue
Copy link
Member

I did not take gov module because I found that latest cosmjs has same MsgVoteWighted . So if I bump up later, it will be the same.

If so, will it be reflected when applying lbm-sdk v0.46.0-rc8?

Copy link
Contributor Author

@loin3 loin3 left a comment

Choose a reason for hiding this comment

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

MsgVoteWeighted is add from cosmjs v0.29.0. So if we bump up to cosmjs v0.29.x when applying lbm-sdk v0.46.0-rc8, it will be ok.

@@ -0,0 +1,233 @@
# Custom Protocol Buffer Codecs
Copy link
Member

Choose a reason for hiding this comment

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

Please check this document to fit our proper explain.

packages/finschia/README.md Outdated Show resolved Hide resolved
packages/finschia/package.json Outdated Show resolved Hide resolved
packages/finschia/package.json Show resolved Hide resolved
packages/finschia/src/finschiaclient.ts Show resolved Hide resolved
@loin3 loin3 requested a review from zemyblue September 21, 2022 09:38
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

Sorry, I think that delete packages/finschia/CUSTOM_PROTOBUF_CODES.md file in this time. And I think it's better to add the document about how to using our lbmjs later.

Comment on lines +107 to +112
// let ret: FT | NFT | null;
// if (token.typeUrl == "/lbm.collection.v1.FT") {
// ret = FT.decode(token.value);
// } else if (token.typeUrl == "/lbm.collection.v1.NFT") {
// ret = NFT.decode(token.value);
// }
Copy link

Choose a reason for hiding this comment

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

Is this a comment I'll need later?

Copy link
Member

Choose a reason for hiding this comment

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

This comment can need it later. So, it is necessary.
Thank you for checking this.

@loin3 loin3 merged commit b261f92 into main Sep 22, 2022
@loin3 loin3 deleted the feat/finshia-client branch September 22, 2022 10:26
@zemyblue zemyblue mentioned this pull request Oct 7, 2022
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.

merge stargate and cosmwasm-stargate into one package
3 participants