-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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 interface definitions for procedure stuff #6488
feat: add interface definitions for procedure stuff #6488
Conversation
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.
It's clear from the comments below that I skimmed a bit too much when reading your design doc previously. I am not asserting that the design should necessarily be changed, but evidently I do think that it could use a bit more scrutiny (and in places justification).
Additionally, having reviewed #6489, I wonder about the return types of many of the
|
Cool cool, good to have context. I understand how this could cause confusion, and I am amenable to changing the return type of
as opposed to:
I (and I think most people?) find that a fluent API is more readable, because each action leads logically to the next.
Like I said, I am amenable to changing This works similarly for our
In generally I appreciate being extremely flexible/cautious with the API. But in this case, I don't think that's a change that's likely to happen. E.g. I can't see a situation where I'd want to return something different from
No, that's what the design doc is for :P |
5ba7faa
to
dab40a4
Compare
The basics
npm run format
andnpm run lint
The details
Resolves
N/A
Proposed Changes
Adds definitions for interfaces related to the shared procedures project.
Reason for Changes
Interfaces are useful for documenting intentions, and they also keep the code flexible.
Test Coverage
N/A
Documentation
N/A
Additional Information
N/A