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

Support for 0.33 #184

Closed
tomtau opened this issue Mar 16, 2020 · 7 comments
Closed

Support for 0.33 #184

tomtau opened this issue Mar 16, 2020 · 7 comments

Comments

@tomtau
Copy link
Contributor

tomtau commented Mar 16, 2020

Primarily Block#Header changed

@liamsi
Copy link
Member

liamsi commented Mar 16, 2020

ref #88 #181 #182

@Shivani912
Copy link
Contributor

#181 (comment)

@greg-szabo
Copy link
Member

In short, Shivani is working on this on branch v0.33. We'll merge that branch to master when we're ready with upgrading to 0.33

@greg-szabo
Copy link
Member

greg-szabo commented Mar 27, 2020

Note: we're planning to use this issue to gather all the outstanding issues that are related to upgrading to v0.33. The issues are usually related to Tendermint Go compatibility over JSONRPC.

Today, I've started going through all RPC struct types and noticed that we sometimes failed to follow the Tendermint-Go implementation's omitempty tags. I'm planning to fix that by using the serde annotation #[serde(default) (thanks @liamsi for mentioning that in another issue), instead of the currently implemented Option<> way.
This creates cleaner looking code and no unwrap() calls.

I'll add other issues here so we track their progress here.

@greg-szabo
Copy link
Member

Related to 0.33 upgrade:

PRs:

Branch:
v0.33 branch

@greg-szabo
Copy link
Member

All issues were resolved in the v0.33 branch. I just need to figure out what to do with the two PRs from @yihuang .

It seems that I've implemented PR #71 in parallel with some differences. I couldn't actually get a query with "code": 1 but all other fields are tested in the integration tests, so if this branch is accepted, #71 can be closed without merge.

As for PR #98, I didn't touch that yet. From a high level, it looks good and I think it's mergeable even with the changes in this branch. I'll check it out.

@greg-szabo
Copy link
Member

All relevant changes are on master. Other issues were opened for refining and improving things.

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

No branches or pull requests

4 participants