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.

Sign up for GitHub

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

Jump to bottom

Extract api definitions #2101

Merged
merged 63 commits into from
Oct 9, 2023
Merged

Extract api definitions #2101

merged 63 commits into from
Oct 9, 2023

Conversation

0xverin
Copy link
Contributor

@0xverin 0xverin commented Sep 4, 2023 •
edited
Loading

Context

resolves #2088

Note: Although this PR involves many file modifications, it's because there were folder movements.Please pay particular attention to the files src/index.ts and dist/src/index.d.ts in the parachain-api/sidechain-api folder.

@0xverin 0xverin force-pushed the 2088-extract-api-definitions branch from 9206b02 to 40a847f Compare September 5, 2023 12:22
@0xverin 0xverin force-pushed the 2088-extract-api-definitions branch from e767053 to 681f064 Compare September 5, 2023 14:47
@0xverin 0xverin force-pushed the 2088-extract-api-definitions branch from e632eef to 8f10a5c Compare September 11, 2023 08:03
@0xverin 0xverin force-pushed the 2088-extract-api-definitions branch from 3731b85 to 4a90114 Compare September 11, 2023 08:35
@0xverin 0xverin force-pushed the 2088-extract-api-definitions branch 2 times, most recently from 189af41 to 2b0b8b0 Compare September 11, 2023 16:28
@0xverin
Copy link
Contributor Author

0xverin commented Sep 28, 2023

We need update docs for this PR, take over to @grumpygreenguy thanks!

@0xverin
Copy link
Contributor Author

0xverin commented Oct 8, 2023

We need update docs for this PR, take over to @grumpygreenguy thanks!

I finished it.

Kailai-Wang
Kailai-Wang reviewed Oct 8, 2023
View reviewed changes
.github/file-filter.yml Outdated
@@ -28,8 +28,10 @@ tee_src: &tee_src
- 'tee-worker/build.Dockerfile'
- 'tee-worker/enclave-runtime/**'
- 'tee-worker/docker/**'
- 'tee-worker/client-api/**'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it intended? Do we need to rebuild the tee image for any file change under this folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have temporarily removed it here. It was added earlier to fix an issue with CI, but now it may not be necessary.

.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
tee-worker/cli/lit_ts_test.sh Outdated Show resolved Hide resolved
grumpygreenguy
grumpygreenguy reviewed Oct 9, 2023
View reviewed changes
docker/yarn.lock Outdated Show resolved Hide resolved
grumpygreenguy
grumpygreenguy reviewed Oct 9, 2023
View reviewed changes
tee-worker/client-api/sidechain-api/dist/package.json Outdated
Comment on lines 10 to 11
"generate-from-defs": "node --experimental-specifier-resolution=node --loader ts-node/esm node_modules/@polkadot/typegen/scripts/polkadot-types-from-defs.mjs --package sidechain-api/interfaces --input build/interfaces --endpoint build/litentry-sidechain-metadata.json",
"generate-from-chain": "node --experimental-specifier-resolution=node --loader ts-node/esm node_modules/@polkadot/typegen/scripts/polkadot-types-from-chain.mjs --package sidechain-api/interfaces --output build/interfaces --endpoint build/litentry-sidechain-metadata.json --strict",
Copy link
Contributor

Choose a reason for hiding this comment

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

Now I'm confused; are we gonna run these with node, tsx, or ts-node? Do we need the ts-node dependency at all? 🤔

(also, should probably invoke it as pmpm exec node to make sure it's using the right environment)

Copy link
Contributor Author

@0xverin 0xverin Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We can add pnpm exec.According to typegen's latest docs, we're better off using node to run. Anyway, after testing, ts-node does not work.

grumpygreenguy
grumpygreenguy reviewed Oct 9, 2023
View reviewed changes
tee-worker/ts-tests/integration-tests/common/type-definitions.ts
@@ -1,4 +1,4 @@
import { ApiPromise } from '@polkadot/api';
import { ApiPromise } from 'parachain-api';
Copy link
Contributor

Choose a reason for hiding this comment

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

<3

grumpygreenguy
grumpygreenguy approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@grumpygreenguy grumpygreenguy left a comment

Choose a reason for hiding this comment

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

Looks good 🙌
Left a few small questions; nothing that looks like a blocker tho

@0xverin 0xverin merged commit f82a6d0 into dev Oct 9, 2023
28 checks passed
@0xverin 0xverin deleted the 2088-extract-api-definitions branch October 9, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kailai-Wang Kailai-Wang Kailai-Wang left review comments

@grumpygreenguy grumpygreenguy grumpygreenguy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Extract API definitions from tee-worker/ts-tests folder
4 participants
@0xverin @Kailai-Wang @grumpygreenguy @kziemianek