-
Notifications
You must be signed in to change notification settings - Fork 39
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
update zoe overview to match dapp-offer-up; add contract basics #889
Conversation
|
Deploying with Cloudflare Pages
|
2531017
to
5bc9dc6
Compare
86103ff
to
6036527
Compare
79f34ae
to
8733a14
Compare
weird. I messed up something with git. hang on... |
ah. I had the wrong target branch. it should be right now. |
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.
This is good.
]), | ||
); | ||
|
||
playerSeat.exit(true); |
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.
shouldn't this also exit()
the tmp
seat as well?
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 don't know. It seems to work ok without doing that. Doesn't letting it go out of scope and get garbage collected suffice?
Checking ZCFSeat docs... all I see is "Note: You should not use aZCFSeat.exit() when exiting with an error."
I thought I had seen this pattern of using a temp seat with mintGains()
, but psm.js seems to use a long-lived seat.
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.
we recently noticed (#8672) that Seats that are not exited can retain some uncollectible (GC-resistant) cycles. Calling .exit()
when you know the seat is no longer in use is now a best practice.
import { start as startState } from '../src/02-state.js'; | ||
import { start as startAccess } from '../src/03-access.js'; | ||
// #region test1 |
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 can see the source code, so I was able to understand the distinction between startAccess
and startState
, but it's not obvious from looking at the generated docs. Does it make sense to add a comment in the tests where they're called?
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 re-factored it to use
import * as state from '../src/02-state.js';
and then state.start()
. I hope that's sufficiently clear.
- don't show region comments - reference to Zoe is just a distraction - index start later - fix test name - don't mention snippets - connect to surrounding sections - link fixes
- debug.js too - refactor(test-alice-trade): rename pmt to Place to match keyword, resulting in short property form that's handy in a diagram - refactor: postpone discussion of harden returning a fresh { ... } record does not put a function at risk. - hide region markers in full listing add whole-file region to suppress others
sections that are starting to take shape: - Bundling a Contract Get this housekeeping aspect out of the way so that further tests can ignore it. - Contract Installation - Starting a contract
- rescue a few bits
- make each section more context-free by harking back explicitly to the basic dapp - focus on lines by number - misc. editorial
Co-authored-by: Chris Hibbert <[email protected]>
611286d
to
96cecd8
Compare
Co-authored-by: Chris Hibbert <[email protected]>
Our getting started material has, to date, been largely disconnected from our Zoe guide. The getting started dapp was a fungible faucet, which had no conditional exchange of assets at all.
The card store app addresses that, but
The thinking here is to use the simple game piece contract, which is somewhat like atomic swap but
Aspects that this contract does not exhibit:
tasks:
fixes #891, #436
refs: