Skip to content

Add back contract deployment#10

Merged
Kay-Zee merged 2 commits into
masterfrom
kan/#2/deploy-contract-on-account-creation
Dec 8, 2020
Merged

Add back contract deployment#10
Kay-Zee merged 2 commits into
masterfrom
kan/#2/deploy-contract-on-account-creation

Conversation

@Kay-Zee
Copy link
Copy Markdown
Member

@Kay-Zee Kay-Zee commented Dec 4, 2020

Closes #2

Description

Adds a --contract flag to allow deploying a contract along with account creation.

How do we feel about the requirement for the flag contents to be name:filepath?


For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@Kay-Zee Kay-Zee requested review from psiemens and turbolent December 4, 2020 06:25
Copy link
Copy Markdown
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for bringing this back

Comment thread flow/accounts/create/create.go Outdated
var contracts []templates.Contract

if len(conf.Contract) > 0 {
contractFlagContent := strings.Split(conf.Contract, ":")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
contractFlagContent := strings.Split(conf.Contract, ":")
contractFlagContent := strings.SplitN(conf.Contract, ":", 2)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I didn't SplitN so that i could use the length as an extra check, but : in filename/path would probably just make it error on read later, which is probably okay.

Comment thread flow/accounts/create/create.go Outdated
HashAlgo string `default:"SHA3_256" flag:"hash-algo" info:"Hash used for the digest"`
Host string `flag:"host" info:"Flow Access API host address"`
Results bool `default:"false" flag:"results" info:"Display the results of the transaction"`
Contract string `flag:"contract,c" info:"Contract to be deployed during account creation. <name:path>"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good idea to use name:path. Maybe make it a slice like Keys above?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

that's a pretty easy way to make it support multiple contracts, so sure! added.

@Kay-Zee
Copy link
Copy Markdown
Member Author

Kay-Zee commented Dec 4, 2020

i was also wondering if it makes sense to go with using the command args, similar to upgrade_contracts.

@Kay-Zee Kay-Zee added the Improvement Technical work without new features, refactoring, improving tests label Dec 4, 2020
@Kay-Zee Kay-Zee marked this pull request as ready for review December 4, 2020 17:43
Copy link
Copy Markdown
Contributor

@psiemens psiemens 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!

I think it's good to make --contracts a flag rather than a positional argument given that it's not required for account creation

@Kay-Zee Kay-Zee merged commit e2a5828 into master Dec 8, 2020
@devbugging devbugging deleted the kan/#2/deploy-contract-on-account-creation branch April 7, 2021 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Improvement Technical work without new features, refactoring, improving tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for deploying contracts during account creation

3 participants