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

Expose contract namespaces under contract.ns.{query, tx}.* #4487

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jacogr
Copy link
Member

@jacogr jacogr commented Jan 21, 2022

No description provided.

@jacogr jacogr added the WIP Work in Progress label Jan 21, 2022
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 {
Copy link

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

Copy link
Member Author

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.

@xgreenx
Copy link

xgreenx commented Feb 25, 2022

When do you plan to merge that change?=)

@jacogr
Copy link
Member Author

jacogr commented Feb 25, 2022

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)

@lgtm-com
Copy link

lgtm-com bot commented Mar 31, 2022

This pull request introduces 2 alerts when merging 089ba53 into 756f817 - view on LGTM.com

new alerts:

  • 2 for Prototype-polluting assignment

@lgtm-com
Copy link

lgtm-com bot commented Apr 9, 2022

This pull request introduces 2 alerts when merging 9c2cdc8 into 6c3bd2f - view on LGTM.com

new alerts:

  • 2 for Prototype-polluting assignment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants