-
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 basic observable implementations of procedure interfaces #6489
feat: add basic observable implementations of procedure interfaces #6489
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.
A preliminary review pending discussion about #6488.
2a6b864
to
76583a0
Compare
09a2dae
to
b3dc7bb
Compare
3d1748e
to
d44b501
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 found this tab spinning, with my approval from yesterday apparently unsubmitted. Grr...
constructor( | ||
private readonly workspace: Workspace, name: string, id?: string) { | ||
this.id = id ?? genUid(); | ||
this.variable = workspace.createVariable(name); |
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.
Shouldn't this try .getVariable(name)
first, like setName
does? And is there a way of doing that by calling setName
—i.e., can tsc infer (or be made to infer) that setName
does the initialisation on behalf of the constructor?
(Aside: the latter is the sort of thing I bet Ezno can handle no problem, no special annotation required…)
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 will look into this.
The basics
npm run format
andnpm run lint
The details
Resolves
Fixes #6514
Dependent on #6488
Proposed Changes
Adds some basic implementations of the procedure interfaces. They don't do any of the important stuff yet (i.e. triggering
doProcedureUpdate
or firing events).Reason for Changes
Interfaces need to be implemented.
Test Coverage
N/A
Documentation
N/A
Additional Information
N/A