-
Notifications
You must be signed in to change notification settings - Fork 13
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
Marketplace: load manifest on demand #896
Conversation
export interface Version { | ||
versionHash: string; | ||
manifest: string; | ||
manifestProtocol: string; | ||
manifestData: Manifest|null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about changing this to manifestData?: Manifest;
and after this change we can set the manifestData directly into https://github.com/mesg-foundation/core/pull/896/files#diff-1497f2b1650c529c8f58159491cfe107R8 instead of recreating the object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel it's safer to have multiple type. Like this, manifestData
doesn't exist in Version and the developer cannot confuse between a not loaded manifestData
and an not existing one (when the file is no more accessible from IPFS).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to update mesg.yml
as well
6f98fe3
to
2284c8c
Compare
Done. Please review |
# Conflicts: # systemservices/marketplace/src/contracts/version.ts # systemservices/marketplace/src/tasks/isAuthorized.ts
# Conflicts: # systemservices/marketplace/src/contracts/version.ts
…tplace-manifest-ondemand-loading # Conflicts: # systemservices/marketplace/mesg.yml
This pr change the way the manifest is downloaded.
Before, getting the version also download the manifest.
Now, the manifest needs to be explicitly downloaded.
Task
getService
still returns the manifest, but tasklistService
doesn'tTODO: