-
Notifications
You must be signed in to change notification settings - Fork 266
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
feat: register-public-key & CLI update to use options instead of args #1397
Conversation
@@ -22,14 +22,27 @@ ROOT=$(pwd) | |||
|
|||
write_import() { | |||
CONTRACT_NAME=$1 | |||
NAME=`echo $CONTRACT_NAME | sed -r 's/(^|_)(.)/\U\2/g'` | |||
|
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.
nicely done
NAME=`echo $CONTRACT_NAME | sed -r 's/(^|_)(.)/\U\2/g'` | ||
|
||
if [ "$(uname)" = "Darwin" ]; then | ||
# sed \U doesn't work on mac |
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.
Can't we just always use perl for simplicity?
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.
yep don't know why I didn't think of 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.
not supported in CI :(
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.
Looks good. Just a minor nitpick.
|
||
- `-a, --address <aztecAddress>`: The account's Aztec address. | ||
- `-p, --public-key <publicKey>`: 'The account public key.' | ||
- `-pa, --partial-address <partialAddress`: 'The partially computed address of the account contract.' |
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.
I would link here the docs Santiago wrote about partial address etc. I feel like these terms are super confusing and I would have no idea what's going on here.
With an Aztec example contract: | ||
|
||
```shell | ||
aztec-cli deploy -c ZkTokenContractAbi -a 333 0x134567890abcdef |
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.
It would be great to add an additional command example-contracts
that simply lists the abi names available. e.g. ZkTokenContractAbi
.
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.
good idea 👍
…ges into sp/cli-updates
Fixes #1297
Fixes #1298
Fixes #1306
Also fix issue with noir-contracts bootstrap on MacOS
Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.