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

Migrate to entry_point macro #249

Merged
merged 4 commits into from
Mar 29, 2021
Merged

Migrate to entry_point macro #249

merged 4 commits into from
Mar 29, 2021

Conversation

maurolacy
Copy link
Contributor

Closes #230.

@maurolacy maurolacy requested a review from ethanfrey March 22, 2021 09:22
@maurolacy maurolacy self-assigned this Mar 22, 2021
Copy link
Member

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

Please don't use the plain #[entry_point] decorator, but rather the library feature flag version. I actually asked Simon to make that the default functionality (disable on library flag), but he thought that was putting too much app-specific logic in cosmwasm base.

If you find the current status too annoying, please look at the original cosmwasm PR; CosmWasm/cosmwasm#701 (comment) and re-open this issue. I just accepted using this larger macro heading everywhere.

contracts/cw1-subkeys/src/contract.rs Outdated Show resolved Hide resolved
contracts/cw1-whitelist/src/contract.rs Show resolved Hide resolved
contracts/cw1-whitelist/src/contract.rs Show resolved Hide resolved
contracts/cw20-bonding/src/contract.rs Outdated Show resolved Hide resolved
@maurolacy
Copy link
Contributor Author

If you find the current status too annoying, please look at the original cosmwasm PR; CosmWasm/cosmwasm#701 (comment) and re-open this issue. I just accepted using this larger macro heading everywhere.

It's OK with me. Maybe the macro can be written in a way that has this check inside? (just curious)

@maurolacy
Copy link
Contributor Author

Will create a new branch to migrate cw-plus to Rust 0.51.0, as there are a number of linting issues to deal with.

@maurolacy maurolacy force-pushed the use-entry_point-macro branch from 190d906 to f124791 Compare March 29, 2021 10:51
@ethanfrey
Copy link
Member

It's OK with me. Maybe the macro can be written in a way that has this check inside? (just curious)

Of course there is, the macro could also emit nothing if the "library" feature flag was set. Simon was worried that we would then be adding "magic" feature flag support into the macros, and said to do that in this repo. I see his point (make it explicit) and your point (make it easy)

@ethanfrey
Copy link
Member

Will create a new branch to migrate cw-plus to Rust 0.51.0, as there are a number of linting issues to deal with.

Sounds good. So, you will do that first and then rebase this on it? I will approve it, so as soon as CI passes, I am happy.

Copy link
Member

@ethanfrey ethanfrey left a comment

Choose a reason for hiding this comment

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

Looks good. As soon as the CI passes (I think the build script just needs to be updated to Rust 1.51)

@maurolacy
Copy link
Contributor Author

maurolacy commented Mar 29, 2021

This was caused by the check_contract dep being built from main instead than from a tag.

Changed it so we can merge this independently of the Rust 1.51.0 migration (thanks @ethanfrey).

@maurolacy maurolacy merged commit 4953039 into master Mar 29, 2021
@maurolacy maurolacy deleted the use-entry_point-macro branch March 29, 2021 16:23
@@ -834,7 +834,7 @@ jobs:
# Uses --debug for compilation speed
# FIXME: Change when `check_contract` (part of `cosmwasm-0.14.0`) is published
#command: cargo install --debug --features iterator --example check_contract -- cosmwasm-vm
command: cargo install --debug --features iterator --git https://github.com/CosmWasm/cosmwasm --branch=main --example check_contract -- cosmwasm-vm
command: cargo install --debug --features iterator --git https://github.com/CosmWasm/cosmwasm --tag=v0.14.0-beta1 --example check_contract -- cosmwasm-vm
Copy link
Member

Choose a reason for hiding this comment

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

🚀

This was referenced Apr 1, 2021
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.

Use #[entry_point] macro in contracts
2 participants