-
Notifications
You must be signed in to change notification settings - Fork 276
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 traits for iterable and non-iterable queries #4638
Conversation
03a3b50
to
5577f25
Compare
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.
LGTM.
I noticed that smart_contract
and client
duplicate impls of query execution, and both have type logic to guard against incorrect use of queries. But this compile-time guard is tested only for client. Would it make sense to make a ui test for smartcontracts as well?
Also, do I understand correctly that the runtime check you are talking about is this code?
I thought about that too, though I just got lazy doing that 😅. I guess I'll update the PR to add that
Yes, this is the check I added in #4544 as per your suggestion =) |
5577f25
to
8260697
Compare
Other than comments LGTM |
81eef3d
to
78320fd
Compare
78320fd
to
ce00459
Compare
Signed-off-by: Nikita Strygin <[email protected]>
…queries This only changes the interfaces (client & smart contract) to be less error-prone, doing so is already a runtime error Signed-off-by: Nikita Strygin <[email protected]>
Signed-off-by: Nikita Strygin <[email protected]>
…racts Signed-off-by: Nikita Strygin <[email protected]>
…racts Signed-off-by: Nikita Strygin <[email protected]>
ce00459
to
22e2041
Compare
Description
This PR is a first step towards more type-safe queries (#4569). It adds traits for singular and iterable queries and prohibits supplying filters, sorting, pagination and batching parameters in compile time (it is already a runtime error to do so).
Linked issue
Partially addresses #4569
Benefits
Checklist