Goal: Introduce new flags to make creating apps without state easier #4946
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4946 +/- ##
==========================================
- Coverage 53.63% 53.60% -0.04%
==========================================
Files 432 432
Lines 54057 54088 +31
==========================================
- Hits 28996 28992 -4
- Misses 22812 22849 +37
+ Partials 2249 2247 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
I've hated the requirement on these switches for these for some time, so I like the idea.
I assume you considered simply not making them required, thus defaulting to 0? That's what I would have pushed for if we were starting from scratch, but maybe you don't feel we should make that change?
The advantage of that is that it lets you conveniently specify only what you care about, rather than needing to specify that the other stuff is 0 as soon as you want to set any.
I'm putting in approval, because this is fine with me if you want to go this way, but I think I'd prefer making them optional instead, even now. After all, since they were required, it can't break any existing scripts/code.
|
Making the schema flags optional would certainly solve the same problem as this PR (and maybe be simpler overall). I guess the question comes down to whether we'd be ok with changing a flag from required to optional. At the moment I don't feel strongly either way. |
| } | ||
| } | ||
|
|
||
| func getStateSchema(cmd *cobra.Command) basics.StateSchemas { |
| printf '#pragma version 2\nint 1' > "${TEMPDIR}/simple.teal" | ||
|
|
||
| # Check goal flags --no-state, --no-local-state, --no-global-state | ||
|
|
There was a problem hiding this comment.
maybe a few extra words here about the methodology
|
This is definitely better in every way. But maybe not better than defaulting them to zero? |
|
After more thought I think I agree it would be better to allow the existing arguments to be optional. If I have time in the next few days, I'll modify this PR to do that instead. |
|
how u doin |
|
Closing this in favor of a future PR which makes the existing arguments optional |
Summary
With the recent introduction of boxes, now more than ever it makes sense for some apps to not use global/local state.
This PR introduces some new convenience flags for
goal app createandgoal method --createwhich makes it easier to create an app without global, local, or neither states. They are:--no-global-state: shorthand for--global-ints=0and--global-byteslices=0--no-local-state: shorthand for--local-ints=0and--local-byteslices=0--no-state: shorthand for--no-local-stateand--no-global-stateFor example, this simplifies usage from
goal app create --creator ${ACCOUNT} --approval-prog ${TEALDIR}/boxes.teal --clear-prog ${TEALDIR}/clear.teal --global-byteslices 0 --global-ints 0 --local-byteslices 0 --local-ints 0togoal app create --creator ${ACCOUNT} --approval-prog ${TEALDIR}/boxes.teal --clear-prog ${TEALDIR}/clear.teal --no-stateTest Plan
New test added in
test/scripts/e2e_subs/goal-app-create-state.sh, and I modified some existing e2e tests to take advantage of the new flags where appropriate.