Skip to content
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

Protocol spec changes #211

Merged
merged 12 commits into from
Mar 30, 2024
Merged

Protocol spec changes #211

merged 12 commits into from
Mar 30, 2024

Conversation

jiyoonie9
Copy link
Contributor

@jiyoonie9 jiyoonie9 commented Mar 26, 2024

blocked by TBD54566975/tbdex#293 merging

This was linked to issues Mar 26, 2024
…assing, but test vectors need to be fixed, also need to look at the new regex pattern decimalString and see if it still works with numbers with decimal
@jiyoonie9 jiyoonie9 changed the title 207 protocol spec changes Protocol spec changes Mar 29, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is what's latest in tbdex main TBD54566975/tbdex@6af20c9

@jiyoonie9
Copy link
Contributor Author

ci will fail - to be addressed by #221

}


@Test
fun parse_quote() {
val vector = TestVectors.getVector("parse-quote.json")
assertNotNull(vector)
testNonErrorMessageTestVector<Quote>(vector)
testSuccessMessageTestVector<Quote>(vector)
}

@Test
fun parse_rfq() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the benefit of other reviews: This is one of the two tests that we expect to fail for this PR. The only other test that should fail is ValidatorTest > validateFailsWithInvalidRfqData(). All other tests should (and do) pass.

Context: #221

Copy link

@diehuxx diehuxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Slick and polished. I pinky promise to fix the two broken tests in this follow up PR #222 very soon

Copy link
Member

@mistermoe mistermoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @jiyoontbd !

@jiyoonie9 jiyoonie9 merged commit 5136956 into main Mar 30, 2024
5 of 6 checks passed
@jiyoonie9 jiyoonie9 deleted the 207-protocol-spec-changes branch March 30, 2024 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Close.success field Uptake Spec Changes
3 participants