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

Change constructor to init(ializer) #381

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Conversation

Neotamandua
Copy link
Member

@Neotamandua Neotamandua commented Aug 21, 2024

The reasons for this change are mainly to improve consistency and also to correctly refer to the init function in the ContractDataBuilder.

Also, my understanding is that the init function is a generic function that is run once during deployment to execute any initialization logic. This implies to me that it can either execute init logic, including initializing data within a contract, but it does not construct the contract, an object, or the state (which already exists after compilation through the static mut STATE in most contracts).

I have also changed the naming within the example contracts. I initially suggested changing constructor_arg to initializer_arg, but opted for init_arg to avoid the z & s ambiguity in UK vs US English.

Copy link
Member

@HDauven HDauven left a comment

Choose a reason for hiding this comment

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

LGTM, but I would wait for @ureeves or @miloszm to also give their blessing.

contracts/Cargo.toml Show resolved Hide resolved
Copy link
Member

@ureeves ureeves left a comment

Choose a reason for hiding this comment

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

Always good to disambiguate! LGTM

- Add deprecation notice for constructor_arg
@Neotamandua Neotamandua merged commit 713a9e7 into main Aug 22, 2024
6 checks passed
@Neotamandua Neotamandua deleted the constructor_to_init branch August 22, 2024 09:34
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.

3 participants