Add deployed field with capture variable, multiple networks #1
Conversation
| as: "SafeMathLibTransactionHash" | ||
| # stop execution and error if we don't get the variable | ||
| # setting to false would mean we continue without it | ||
| strict: true |
There was a problem hiding this comment.
🎨 Not super opinionated here, but between strict and the alternative, loose mentioned in the README, I personally prefer strict. I'm not sure how to articulate it, but something about the capture context here makes loose feel potentially confusing. Open to push back though!
There was a problem hiding this comment.
I think I agree! It feels clearer. I think @gnidan had advocated for loose. Open to discussing it of course!
There was a problem hiding this comment.
can we make the capture syntax a separate PR? seems like it's more complicated than we should include here
| # put npm packages needed for deployment here | ||
| require: ["dotenv"] | ||
| # organize by network, then by | ||
| deployed: |
There was a problem hiding this comment.
❓ I know there was conversation around potentially letting users define various states: (deployed, maintenance, beta etc), but I'm wondering if we can just make the deployed state our MVP and allow for custom states later.
There was a problem hiding this comment.
I think that is reasonable. Then the initial solver can just deal with the base state of deployed. I'm not sure I'd want the other options at all honestly -- a user can version their yaml files and have the deployed state be something different in each version maybe. It feels like what is "deployed" is the main concern of the solver and these other states could muddy that. Definitely open to discussing that further but we can probably all agree to start with deployed!
| HumanStandardToken: | ||
| # setting ifTrue tells the solver not to proceed with this deployment unless | ||
| # we have the designated capture variable. ifTrue could also be a list/array | ||
| ifTrue: SafeMathLibAddress |
There was a problem hiding this comment.
❓ Since we will be supporting strict from the SafeMathLib contract side, do we need to support this conditional logic from dependents of SafeMathLib? I guess having both would offer flexibility for a scenario like:
SomeLibAddress strict: false to ignore deploy failures (perhaps to allow non-dependant contracts to continue deploying), but then SomeDependent set to ifTrue: SomeLibAddress blow up.
| arguments: | ||
| - SafeMathLib | ||
| - options: | ||
| gasPrice: 15000000 |
There was a problem hiding this comment.
| gasPrice: 15000000 | |
| gasPrice: 15000000 |
| as: "SafeMathLibTransactionHash" | ||
| # stop execution and error if we don't get the variable | ||
| # setting to false would mean we continue without it | ||
| strict: true |
There was a problem hiding this comment.
can we make the capture syntax a separate PR? seems like it's more complicated than we should include here
| version: 0.0.1 | ||
| description: declarative deployment file "declarative deployments for Truffle projects | ||
| # put npm packages needed for deployment here | ||
| require: ["dotenv"] |
There was a problem hiding this comment.
how do you see the flow for requiring external packages? do you not think it should be in the declaration yaml file? this was a first pass at doing that
| @@ -0,0 +1,38 @@ | |||
| version: 0.0.1 | |||
There was a problem hiding this comment.
I was thinking the version of the DApp? I guess we should talk about what versions of what need to be indicated here.
| @@ -0,0 +1,38 @@ | |||
| version: 0.0.1 | |||
| description: declarative deployment file "declarative deployments for Truffle projects | |||
There was a problem hiding this comment.
what's the purpose of this description field?
This PR adds a
deployedelement to ouryamlfile, organized by network.A couple of other features included here: