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.

Sign up for GitHub

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

Jump to bottom

Add `remove` command #837

Merged

ascjones merged 38 commits into master from ln-add-remove-command

Feb 15, 2023

Contributor

lean-apple commented Nov 22, 2022 •

edited

Loading

In the wake of this issue, #671

ToDo :

Improve how to get code_hash
Add Tests with arg and with the manifest path only
Update documentation

lean-apple added 4 commits

November 22, 2022 12:07


          first version of remove command integration

b7e587a


          import remove command

810661a


          fix some type errors for code_hash

4b86a21


          update with last changes

d6c41d9

ascjones reviewed

View reviewed changes

Collaborator

ascjones left a comment

Can see it's still a WIP but looks reasonable so far. The main thing to check is about that dry-run endpoint on pallet-contracts. We may want to consider that it is not necessary in this case to do the dry-run since we don't need the gas limit estimate like we do with other commands. The only thing to think about is how to elegantly handle the case where the code_has to be removed does not exist on chain.

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

lean-apple added 3 commits

December 14, 2022 17:35


          optimize parse_code_hash function use

08d5198


          remove rpc call and code use

ebeecd5


          Merge branch 'master' into ln-add-remove-command

9fa4706

Collaborator

ascjones commented Jan 11, 2023

Once #893 is merged we could use that too for determining the code hash

lean-apple added 2 commits

January 16, 2023 13:15


          get last changes

ea7f284


          use new way to get code_hash

f07ee02

lean-apple changed the title ~~Add remove command~~ Add remove command

lean-apple added 10 commits

January 19, 2023 04:14


          fix event for remove command result

a3fea1d


          fix fmt

f27a4ee


          change the way to get code hash


          add last changes

2f2f68c


          add remove command integration test

def1ade


          fix fmt

33a6662


          Merge branch 'master' into ln-add-remove-command

05d9e75


          ignore integration tests

27e25b9


          complete documentation for remove command

2b2ce84


          remove unused parse code hash function

d94be80

lean-apple marked this pull request as ready for review

January 25, 2023 17:07

lean-apple requested review from cmichi and HCastano as code owners

January 25, 2023 17:07

ascjones reviewed

View reviewed changes

Collaborator

ascjones left a comment

Looks good, I think we need the optional --code-hash arg back though.

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

docs/extrinsics.md Outdated Show resolved Hide resolved

lean-apple added 2 commits

January 27, 2023 13:16


          fix: draft for code_hash arg return

67e5a77


          complete remove command documentation with code-hash

dcccb12

ascjones reviewed

View reviewed changes

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

ascjones reviewed

View reviewed changes

crates/transcode/src/decode.rs Outdated Show resolved Hide resolved

lean-apple added 4 commits

January 27, 2023 15:20


          fix previous merge of last changes

4aee17f


          fix previous merge of last changes

bb30200


          fix previous merge of last changes


          fix: fix code_hash check but still error variant issue

9daca4c

ascjones reviewed

View reviewed changes

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

lean-apple added 2 commits

February 2, 2023 11:45


          Merge branch 'master' into ln-add-remove-command

c9fc21c


          fix code-hash handle

e80644a

lean-apple requested a review from SkymanOne as a code owner

February 3, 2023 19:28

SkymanOne approved these changes

View reviewed changes

Contributor

SkymanOne left a comment

LGTM! Just a few nitpicks :)
Also, should we have a confirmation just in case?

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved


          get last changes

caf4879

ascjones reviewed

View reviewed changes

Collaborator

ascjones left a comment

Just a few suggestions but we are nearly there!

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

docs/extrinsics.md Show resolved Hide resolved

docs/extrinsics.md Outdated Show resolved Hide resolved

lean-apple added 2 commits

February 13, 2023 16:43


          optimize get hash and error message

6058aa8


          fix documentation

bd958be

ascjones reviewed

View reviewed changes

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved


          improve code_hash

ascjones reviewed

View reviewed changes

Collaborator

ascjones left a comment

Please address the final couple of comments then we can merge 👍

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

crates/cargo-contract/src/cmd/extrinsics/remove.rs Outdated Show resolved Hide resolved

lean-apple and others added 7 commits

February 14, 2023 18:32


          fix error handling


          Add api imports

a857219


          Use correct event type

a39c712


          Fix extrinsics.md

cba704b


          Fix extrinsics.md

c733739


          Wrap long error message

27763a2


          Error message formatting

59117d8

ascjones approved these changes

View reviewed changes

Collaborator

ascjones left a comment

🎉

Contributor Author

lean-apple commented Feb 15, 2023

Thanks for the final touches

ascjones merged commit 901d805 into master

ascjones deleted the ln-add-remove-command branch

February 15, 2023 11:47

This was referenced Feb 15, 2023

Add remove_code extrinsic command #671

Closed

Release v2.0.0 #970

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet