BREAKING CHANGE: remove old amountMath #2894
Conversation
b74a863 to
f6dbb33
Compare
5979e05 to
963a8a1
Compare
|
@erights, I think the majority of the review is for you but @michaelfig could you check on the wallet and any other code you're the owner for? And @FUDCo I made some changes to swingset-runner tests. Thanks! |
|
Adding @Chris-Hibbert to take over part or all of @erights's review. @Chris-Hibbert, can you check ERTP and Zoe especially? Thanks! |
|
|
||
| const result = await Promise.all([emptyPurseMadeP, localAmountMathSavedP]); | ||
| const result = await emptyPurseMadeP; |
There was a problem hiding this comment.
might as well get rid of emptyPurseMadeP, and await makeEmptyPurse(). Actually, can't the log message go before the await, and this can just return the awaited value?
There was a problem hiding this comment.
Yup, I think that's right
| const { offer } = makeOfferAndFindInvitationAmount( | ||
| walletAdmin, | ||
| zoe, | ||
| zoeInvitationPurse, | ||
| getLocalAmountMath, | ||
| invitationMath, | ||
| invitationBrand, |
There was a problem hiding this comment.
It doesn't look like brand was added to makeOfferAndFindInvitationAmount.
e53ccb4 to
a44edb1
Compare
| @@ -166,7 +166,7 @@ test('lib-wallet issuer and purse methods', async t => { | |||
| const moolaPurse = wallet.getPurse('fun money'); | |||
| t.deepEqual( | |||
| await moolaPurse.getCurrentAmount(), | |||
| moolaBundle.amountMath.getEmpty(), | |||
| amountMath.makeEmpty(moolaBundle.brand, MathKind.NAT), | |||
There was a problem hiding this comment.
Minor: This code would be easier to manage if the brands were pulled out into their own variables?
There was a problem hiding this comment.
I think that's out of scope for this PR, and also it's a test so refactoring is probably lower priority.
packages/deploy-script-support/test/unitTests/test-findInvitationAmount.js
Outdated
Show resolved
Hide resolved
0c49ca9 to
fa884ba
Compare
Merging is blocked by removing the old amountMath in the dapps (#2603), but we can still get it reviewed.