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

Telescope upgrade #44

Merged
merged 40 commits into from Oct 26, 2022
Merged

Telescope upgrade #44

merged 40 commits into from Oct 26, 2022

Conversation

pyramation
Copy link
Contributor

No description provided.

@pyramation pyramation mentioned this pull request Oct 12, 2022
@pyramation
Copy link
Contributor Author

pyramation commented Oct 12, 2022

Fixed all the comments, casing, and issues from before, should be in better shape!

@webmaster128
Copy link
Member

do we need to include the ICA types from #36

Yeah, let's include them. You find them in wasmd-0.28/third_party/proto/ibc/applications/interchain_accounts now

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Amazing! Let's get the ICA types in here (rebase to latest main and do a git submodule init && git submodule update then you have them). Otherwise 🏅

"Will Clark <willclarktech@users.noreply.github.com>"
"Simon Warta <webmaster128@users.noreply.github.com>",
"Will Clark <willclarktech@users.noreply.github.com>",
"Dan Lynch <pyramation@gmail.com>"
Copy link
Member

Choose a reason for hiding this comment

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

🥰

* telescope-upgrade: (25 commits)
  context fix import as
  fix Misbehavior
  pagination
  field optionality
  optionality
  tslint disable
  RPC comments
  RPC interface comments
  toJSON Duration
  enum spacing
  enum spacing
  enums
  upgrade Duration types
  upgrade
  fromJSON
  eslint-disable
  eslintDisable
  run codegen
  update
  codegen settings
  ...
@pyramation
Copy link
Contributor Author

pyramation commented Oct 12, 2022

the check is giving:

src/cosmos/tx/v1beta1/service.ts:924:14 - error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.

I think I know how to solve this, but will make a large diff.

UPDATE: I'm able to fix this, it does make a large diff. See comments below.

@pyramation
Copy link
Contributor Author

pyramation commented Oct 13, 2022

the check is giving:

src/cosmos/tx/v1beta1/service.ts:924:14 - error TS7056: The inferred type of this node exceeds the maximum length the compiler will serialize. An explicit type annotation is needed.

I think I know how to solve this, but will make a large diff.

https://app.circleci.com/pipelines/github/confio/cosmjs-types/123/workflows/569fd5f5-81d4-49c0-b0ec-27d44cbead66/jobs/115

btw — we can try doing this for only the google module if you want to keep the Exact<DeepPartial business. The plugin system should support that.

I found this error before and it's the meta typing (that I found quite unnecessary) that causes the memory issues.

FYI seems that you were around when they did this stuff and may have more context:

other related issues/prs

@pyramation
Copy link
Contributor Author

pyramation commented Oct 14, 2022

OK, I think that may be it!

Only difference should be Exact vs DeepPartial, etc., but this method works and is much faster for the TS compiler to deal with.

We can roll back the Exact/DeepPartial stuff, but then the CI can't process things because of TS compiler memory.

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Alright, let's give this a shot. I created a branch 0.5 for patching the old setup as long as we need it. This goes to main and 0.6.0-alpha release such that we can start testing it.

Amazing stuff 👏 Thanks a lot, Dan!

@webmaster128 webmaster128 merged commit 2f5736e into confio:main Oct 26, 2022
@pyramation
Copy link
Contributor Author

wow this marks the end of an era! and a new beginning. Excited to start improving things like DecCoin and Long 🙌🏻

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.

None yet

2 participants