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

Feat/standardize web3api manifest #50

Merged
merged 29 commits into from
Nov 30, 2020

Conversation

cbrzn
Copy link
Contributor

@cbrzn cbrzn commented Oct 19, 2020

closes: #17

@cbrzn cbrzn requested a review from dOrgJelli October 19, 2020 23:16
@cbrzn cbrzn changed the title Feat/standardize web3api manifest [WIP] Feat/standardize web3api manifest Oct 19, 2020
@cbrzn cbrzn marked this pull request as draft October 19, 2020 23:17
packages/cli/tsconfig.json Outdated Show resolved Hide resolved
demos/simple-storage/protocol/Manifest.ts Outdated Show resolved Hide resolved
demos/simple-storage/protocol/web3api.yaml Outdated Show resolved Hide resolved
packages/cli/src/lib/Compiler.ts Outdated Show resolved Hide resolved
packages/cli/src/lib/Manifest.ts Outdated Show resolved Hide resolved
packages/cli/src/lib/Manifest.ts Outdated Show resolved Hide resolved
packages/cli/src/lib/Web3API.ts Outdated Show resolved Hide resolved
packages/cli/src/lib/helpers/schema.json Outdated Show resolved Hide resolved
packages/cli/src/lib/helpers/validators.ts Outdated Show resolved Hide resolved
packages/cli/tsconfig.json Outdated Show resolved Hide resolved
@cbrzn
Copy link
Contributor Author

cbrzn commented Oct 25, 2020

Did some changes. Feel free to check it - Anyways, let's talk about this on tuesday's call :-D @dOrgJelli

@cbrzn cbrzn requested a review from dOrgJelli October 25, 2020 17:54
@cbrzn
Copy link
Contributor Author

cbrzn commented Oct 26, 2020

Implemented a PoC of the migrator. Right now we are using any for the Schema object and we need to use the Manifest type which is autogenerated based on the JSON schema. Let's discuss when possible when should we generate this file and where.

@cbrzn
Copy link
Contributor Author

cbrzn commented Oct 28, 2020

After making the Manifest file autogenerated, I've come to some thought about what is the CLI expecting from this file... I would like to discuss this further, to make sure we are on the same page on how the structure of this manifest is going to be

// - validate manifest structure
// - ensure everything exists

manifestValidation(manifest)
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 think that here, we should redefine the manifest variable. The reason is that on this function, first, we check if the manifest passed as argument is outdated, if that's true, then it's going to be updated, hence, editing this variable that is being passed. If we don't update the manifest variable here, it will compile the older version of the manifest.

Copy link
Contributor

Choose a reason for hiding this comment

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

Signature should be something like this: manifestValidation(manifest: AnyManifest): LatestManifest, so we can set manifest equal to return value.

Also change the function name to sanitizeAndUpgrade

Copy link
Contributor

Choose a reason for hiding this comment

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

Also change the import path from root level to some nested folder like @web3api/client-js/manifest, this way top level imports is very minimal and easy to understand.

@cbrzn cbrzn force-pushed the feat/standardize-web3api-manifest branch from 9660bb4 to cd76b39 Compare October 28, 2020 01:20
@cbrzn cbrzn force-pushed the feat/standardize-web3api-manifest branch from cd76b39 to e56a422 Compare October 28, 2020 01:20
@cbrzn cbrzn marked this pull request as ready for review November 10, 2020 01:10
@cbrzn cbrzn changed the title [WIP] Feat/standardize web3api manifest Feat/standardize web3api manifest Nov 10, 2020
// - validate manifest structure
// - ensure everything exists

manifestValidation(manifest)
Copy link
Contributor

Choose a reason for hiding this comment

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

Signature should be something like this: manifestValidation(manifest: AnyManifest): LatestManifest, so we can set manifest equal to return value.

Also change the function name to sanitizeAndUpgrade

// - validate manifest structure
// - ensure everything exists

manifestValidation(manifest)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also change the import path from root level to some nested folder like @web3api/client-js/manifest, this way top level imports is very minimal and easy to understand.

packages/client-js/src/Web3API.ts Outdated Show resolved Hide resolved
@cbrzn
Copy link
Contributor Author

cbrzn commented Nov 17, 2020

Also change the import path from root level to some nested folder like @web3api/client-js/manifest, this way top level imports is very minimal and easy to understand.

Regarding that point i've been doing some research and came up with this: microsoft/TypeScript#8305

Looks like there is no an official way to do this. Reading that thread you will come up to this: microsoft/TypeScript#33079 but not sure how should we tackle this? @dOrgJelli

@dOrgJelli dOrgJelli merged commit bc3bdd4 into master Nov 30, 2020
@dOrgJelli dOrgJelli deleted the feat/standardize-web3api-manifest branch December 3, 2020 19:07
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.

Standardize Web3API Manifest Schema
2 participants