-
Notifications
You must be signed in to change notification settings - Fork 355
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
Expose contract namespaces under contract.ns.{query, tx}.* #4487
base: master
Are you sure you want to change the base?
Conversation
return call; | ||
} | ||
|
||
export function expandConstructors <ApiType extends ApiTypes, R extends ISubmittableResult> (constructors: AbiMessage[], ns: Namespaced<BlueprintDeploy<ApiType>>, tx: MapConstructorExec<ApiType>, creator: (constructorOrId: AbiConstructor | string | number, options: BlueprintOptions, params: unknown[]) => SubmittableExtrinsic<ApiType, R>): void { |
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.
At the moment ink! doesn't plan to support namespaced constructors, so maybe we can remove that
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.
Consistency. It is easier when it is exactly the same everywhere from a maintenance perspective.
If in this case the namespace is single level, so be it. If it changes, it is catered for. Importantly- there are no implementation differences.
When do you plan to merge that change?=) |
There needs to be a full end-to-end test done with the apps UI for V1, V2 & V3 contracts with this change in. Once verified, it can be merged. (The same applies to the other PRs that are marked WIP, they look sane but have not been tested end-to-end) |
This pull request introduces 2 alerts when merging 089ba53 into 756f817 - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging 9c2cdc8 into 6c3bd2f - view on LGTM.com new alerts:
|
No description provided.