-
Notifications
You must be signed in to change notification settings - Fork 42
docs: update contract walkthru #1176
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
Conversation
Deploying documentation with
|
Latest commit: |
929aa46
|
Status: | ✅ Deploy successful! |
Preview URL: | https://2ef761d6.documentation-7tp.pages.dev |
Branch Preview URL: | https://mk-offer-up-doc-updates.documentation-7tp.pages.dev |
Cloudflare deployment logs are available here |
568fe4c
to
f8b60c1
Compare
@amessbee could you please take a look at this? :) Would appreciate any feedback. |
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.
excellent work!
any particular reason why it's still DRAFT?
Also, it looks like it closes the issue.
* this contract is parameterized by terms for price and, | ||
* optionally, a maximum number of items sold for that price (default: 3). | ||
* | ||
* @typedef {{ |
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 wonder why GitHub actions is issuing a warning here:
Syntax error in type: {tradePrice: Amount;maxItems?: bigint;}
I wonder if some tsc
version is out of date somewhere.
I'm not too worried about it just now, though.
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.
ah. found one tweak
Zoe--)Zoe: escrow Price pmt | ||
|
||
Zoe--)-C: joinHandler(gameSeat) | ||
Zoe--)-C: tradeHandler(proceeds) |
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 like proceeds
got mixed up with buyerSeat
.
Zoe--)-C: tradeHandler(proceeds) | |
Zoe--)-C: tradeHandler(buyerSeat) |
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.
gameSeat
is being referred to as proceeds
in the current version of contract (the seat created via makeEmptySeatKit
). Or am I interpreting it wrong? Or was it a typo in the previous iteration of the diagram?
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.
@dckc PTAL at this^
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.
was it a typo in the previous iteration of the diagram?
yes, I suppose it was.
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.
Cool. Then I'll fix this for trade-offer-safety-4 as well. Thanks!
Do a Draft PR and have it reviewed by others (as per CONTRIBUTING.md). |
interesting. you're very thorough indeed. I glazed over that part. I operate under the flip side of...
I/we should probably change CONTRIBUTING.md (in a later PR). In most of the shop, we don't necessarily do a DRAFT. In fact, IME the norm is to not invite review until it's non-DRAFT. |
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.
nice work!
a8bff9b
to
929aa46
Compare
Closes: #1173
Description
This PR: