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

feat: add t0123 ipns #59

Merged
merged 8 commits into from
Jun 8, 2023
Merged

feat: add t0123 ipns #59

merged 8 commits into from
Jun 8, 2023

Conversation

laurentsenta
Copy link
Contributor

@laurentsenta laurentsenta commented May 29, 2023

  • build the test
  • add a system to run 2 queries and compare the outputs

@laurentsenta laurentsenta mentioned this pull request May 29, 2023
20 tasks
@laurentsenta laurentsenta marked this pull request as draft May 29, 2023 15:33
@laurentsenta laurentsenta marked this pull request as ready for review May 30, 2023 12:01
Copy link
Contributor

@galargh galargh left a comment

Choose a reason for hiding this comment

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

I'm cool with proceeding with this as-is until we get more use cases for multi-request/multi-response setups.

Some ideas/thoughts I got looking at it:

  • Should a test struct have both Request and Requests prop on it? Could we get away with having one?
  • Do we want to limit duplication if parts of multiple requests are the same? As in, should we allow, for example, Query method on the RequestBuilder list and have it set query for all the requests?
  • Similarly for the expects, would it be useful if we were able to do all the checks that we have on ExpectBuilder also on ExpectsBuidlers?
  • This one's a bit unrelated but would it make sense for SugarTest to be a builder as well?

Just wanted to dump ^ before I forget but no need to dig into it now.

Approving provided Henrique's comment is resolved ✅

@laurentsenta laurentsenta merged commit bc93bae into main Jun 8, 2023
@laurentsenta laurentsenta deleted the t0123-ipns branch June 8, 2023 09:07
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.

3 participants