-
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: procedure callers respond to models #6718
feat: procedure callers respond to models #6718
Conversation
1f9163b
to
c53a4a1
Compare
c53a4a1
to
4e79d35
Compare
346e097
to
8c688b4
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.
I have been investigating unexpected diagnostics appearing in npm test
output. Several were traced to two minor errors in this PR.
|
||
/** | ||
* @param {string} name The name of the procedure model to find. | ||
* @param {string[]} params The param names of the procedure model to find. |
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.
string[]
is not valid Closure type syntax. Do you mean Array<string>
?
This provoked a warning from closure-make-deps
.
* Creates a procedure definition block with the given name and params, | ||
* and returns the procedure model associated with it. | ||
* @param {string} name The name of the procedure to create. | ||
* @param {string[]} params The names of the parameters to create. |
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.
Ditto.
Can fix! |
The basics
npm run format
andnpm run lint
The details
Resolves
Work on #6526
Proposed Changes
Refactors the procedure caller blocks to rely on the backing data models.
Reason for Changes
Using the data models makes it so that procedures are explicitly defined!
Test Coverage
Enabled some tests, added some tests, reorganized some tests etc.
Documentation
N/A
Additional Information
Almost certainly best to review this commit-wise haha.
Dependent on #6706