Update the offering section of the readme to match types.js
#116
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SiennaSaito
requested review from
michaelneale,
mistermoe,
jiyoontbd,
phoebe-lew and
KendallWeihe
as code owners
SiennaSaito
force-pushed
the
sienna/update-offering-section-in-readme
branch
from
5f39c73
to
96d60f4
Compare
SiennaSaito
commented
Jul 12, 2023
README.md
Outdated
| | ----------------------- | ------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------ | | ||
| | `id` | string | Y | Unique identifier for this offering | | ||
| | `description` | string | Y | Brief description of what is being offered. | | ||
| | `quoteUnitsPerBaseUnit` | string | Y | ??? | |
There was a problem hiding this comment.
Not sure what to use for the description for quoteUnitsPerBaseUnit 🤔
SiennaSaito
force-pushed
the
sienna/update-offering-section-in-readme
branch
from
96d60f4
to
c3f022d
Compare
|
Thank you so much for writing this PR! I can go over it tomorrow since I’m
about to sleep but if you’d like some additional context, please feel free
to check out the tbdex-types-examples.ts file in the meantime. It might
give you a better sense of the purpose of each type.
…On Tue, Jul 11, 2023 at 11:59 PM Sienna ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In README.md
<#116 (comment)>
:
> -| `baseCurrency` | string | Y | Currency that the PFI is selling. |
-| `quoteCurrency` | string | Y | Currency that the PFI is accepting as payment for `baseCurrency`. |
-| `unitPriceDollars`| string | Y | How much `quoteCurrency` is required for PFI to sell 1 unit of `baseCurrency`. |
-| `baseFeeDollars` | string | N | Optional base fee associated with this offering, regardless of which payment methods are used |
-| `minDollars` | string | Y | Minimum amount of quote currency that the counterparty (Alice) must submit in order to qualify for this offering. |
-| `maxDollars` | string | Y | Maximum amount of quote currency that the counterparty (Alice) can submit in order to qualify for this offering. |
-| `kycRequirements` | string | Y | PresentationDefinition in JWT string format which describes the credential needed to choose this offer. |
-| `payinMethods` | list[PaymentMethod] | Y | A list of payment methods the counterparty (Alice) can choose to send payment to the PFI from in order to qualify for this offering. |
-| `payoutMethods` | list[PaymentMethod] | Y | A list of payment methods the counterparty (Alice) can choose to receive payment from the PFI in order to qualify for this offering. |
-| `createdTime` | datetime | Y | The creation time of the resource. Expressed as ISO8601 |
-
+| field | data type | required | description |
+| ----------------------- | ------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------ |
+| `id` | string | Y | Unique identifier for this offering |
+| `description` | string | Y | Brief description of what is being offered. |
+| `quoteUnitsPerBaseUnit` | string | Y | ??? |
Not sure what to use for the description here either.
—
Reply to this email directly, view it on GitHub
<#116 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A4IFEF5M3F64LDVLMK5IK4TXPZDOZANCNFSM6AAAAAA2G7PBRY>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
phoebe-lew
reviewed
Jul 12, 2023
README.md
Outdated
| | ----------------------- | ------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------ | | ||
| | `id` | string | Y | Unique identifier for this offering | | ||
| | `description` | string | Y | Brief description of what is being offered. | | ||
| | `quoteUnitsPerBaseUnit` | string | Y | ??? | |
There was a problem hiding this comment.
Suggested change
| | `quoteUnitsPerBaseUnit` | string | Y | ??? | | |
| | `quoteUnitsPerBaseUnit` | string | Y | Number of quote units on offer for one base currency unit (i.e 290000 USD for 1 BTC | |
phoebe-lew
reviewed
Jul 12, 2023
README.md
Outdated
| @@ -67,32 +63,32 @@ Base Currency is the currency that the PFI is **selling**. Quote currency is the | |||
| | field | data type | required | description | | |||
| | ---------------------------------- | --------- | -------- | --------------------------------------------------------------------------------------------------- | | |||
| | `kind` | string | Y | Type of payment method (i.e. `DEBIT_CARD`, `BITCOIN_ADDRESS`, `SQUARE_PAY`) | | |||
| | `paymentPresentationDefinitionJwt` | string | N | PresentationDefinition that describes the VCs needed to use this PaymentMethod in JWT string format | | |||
| | `requiredPaymentDetails` | string | N | ??? | | |||
There was a problem hiding this comment.
Suggested change
| | `requiredPaymentDetails` | string | N | ??? | | |
| | `requiredPaymentDetails` | string | N | Details of the VCs required to use this payment in json-schema format | |
phoebe-lew
reviewed
Jul 12, 2023
README.md
Outdated
| "feeSubunits": "100" | ||
| }, | ||
| { | ||
| "kind": "SQUARE_PAY", | ||
| "paymentPresentationDefinitionJwt": null, | ||
| "requiredPaymentDetails": ???, |
There was a problem hiding this comment.
Suggested change
| "requiredPaymentDetails": ???, |
phoebe-lew
reviewed
Jul 12, 2023
README.md
Outdated
| "kind": "BTC_ADDRESS", | ||
| "paymentPresentationDefinitionJwt": "abc...IDsx" | ||
| "requiredPaymentDetails": ??? |
There was a problem hiding this comment.
Suggested change
| "requiredPaymentDetails": ??? | |
| "requiredPaymentDetails": { | |
| "$schema": "http://json-schema.org/draft-07/schema#", | |
| "title": "BTC Required Payment Details", | |
| "type": "object", | |
| "required": [ | |
| "btcAddress" | |
| ], | |
| "additionalProperties": false, | |
| "properties": { | |
| "btcAddress": { | |
| "description": "The address you wish to receive BTC in", | |
| "type": "string" | |
| } | |
| } | |
| } |
indenting might be a bit wonky!
phoebe-lew
reviewed
Jul 12, 2023
README.md
Outdated
| | `feeSubunits` | string | N | Optional fee associated with using this kind of payment method in the smallest currency delineation | | ||
|
|
||
|
|
||
| ```json | ||
| { | ||
| "id": ???, |
There was a problem hiding this comment.
Suggested change
| "id": ???, | |
| "id": "tbdex:offering:123456", |
phoebe-lew
reviewed
Jul 12, 2023
README.md
Outdated
| "description": "Buy BTC with USD!", | ||
| "quoteUnitsPerBaseUnit": ???, |
There was a problem hiding this comment.
Suggested change
| "quoteUnitsPerBaseUnit": ???, | |
| "quoteUnitsPerBaseUnit": "29000", |
phoebe-lew
approved these changes
Jul 12, 2023
…'s inside types.js
SiennaSaito
force-pushed
the
sienna/update-offering-section-in-readme
branch
from
c3f022d
to
a6c82d7
Compare
|
Thanks for the quick review @phoebe-lew + @jiyoontbd! 😎 ✨ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
The offering section of the readme is out of date with
types.ts.This PR manually updates the readme. We'll be able to automate this work if #115 or something similar happens but I want to get the readme up to date so we have something to validate automatic generation against.
References removed:
References added:
Remaining work
I actually don't know anything about TBDex so I have no idea what any of the properties in Offering do.
I've marked each entry that is missing a description with ???
If someone could comment inline on the files changed with what each ??? should be replaced with that would be super cool!