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

fix(vat-data)!: deprecate kinds in favor of Far Classes #6106

Merged
merged 3 commits into from
Nov 9, 2022

Conversation

erights
Copy link
Member

@erights erights commented Sep 1, 2022

I believe we've now had enough experience with FarClasses, etc, to deprecate the virtual/durable kind APIs and unconditionally recommend FarClasses, etc, instead.

@erights erights self-assigned this Sep 1, 2022
@erights erights requested a review from turadg September 3, 2022 11:17
Base automatically changed from markm-far-classes to master September 3, 2022 22:48
@erights erights force-pushed the markm-deprecate-kinds branch 5 times, most recently from d9469b9 to e376354 Compare September 9, 2022 22:37
@erights erights force-pushed the markm-deprecate-kinds branch 2 times, most recently from 4765972 to 73086ad Compare September 17, 2022 04:46
@erights erights changed the title WIP deprecate kinds in favor of Far Classes fix(vat-data)!: deprecate kinds in favor of Far Classes Sep 19, 2022
@erights erights marked this pull request as ready for review September 19, 2022 01:37
@turadg
Copy link
Member

turadg commented Sep 22, 2022

@erights you marked this Ready for Review but the PR description says, "We won't know if this is a good idea until…". Do you think it's a good idea now?

@erights
Copy link
Member Author

erights commented Sep 22, 2022

@erights you marked this Ready for Review but the PR description says, "We won't know if this is a good idea until…". Do you think it's a good idea now?

Good question! You and @Chris-Hibbert have given these more road testing at this point than I have. What do you both think? Are we ready to deprecate kinds in favor of far classes / far instances?

@Chris-Hibbert
Copy link
Contributor

I don't know enough yet to think we should definitely start on a project to convert everything over, but I prefer Far classes at this point. I'm not sure whether it would be more prudent to finish making Zoe durable using Kinds, rather than mixing the two styles.

@erights erights force-pushed the markm-deprecate-kinds branch 3 times, most recently from 225e877 to bf4a71a Compare September 29, 2022 03:38
@turadg turadg removed their request for review October 4, 2022 16:09
@erights erights force-pushed the markm-deprecate-kinds branch 2 times, most recently from aa7da76 to 2fe0538 Compare October 9, 2022 04:25
@erights erights requested a review from FUDCo October 11, 2022 00:53
@erights erights requested a review from mhofman October 11, 2022 23:55
@erights erights force-pushed the markm-deprecate-kinds branch 3 times, most recently from 440cb4c to 91eab30 Compare October 20, 2022 20:42
@erights
Copy link
Member Author

erights commented Nov 5, 2022

Reviewers (as well as @turadg @michaelfig ), I'd appreciate help in how to deprecate these so that the deprecations show up more reliably under vscode. Thanks.

@turadg
Copy link
Member

turadg commented Nov 7, 2022

help in how to deprecate these so that the deprecations show up more reliably under vscode

The export const is implicitly defining a new types. I don't see a way to to propagate the types but we can mark the new one as deprecated.


/** @deprecated */
 export const {
   defineKind,
   defineKindMulti,
   defineDurableKind,
   defineDurableKindMulti,
} = VatDataGlobal;

export const {
   makeKindHandle,
   providePromiseWatcher,

@erights
Copy link
Member Author

erights commented Nov 8, 2022

Re #6106 (comment) , Done.

@erights erights requested a review from turadg November 8, 2022 00:29
@erights erights added the automerge:squash Automatically squash merge label Nov 9, 2022
@mergify mergify bot merged commit b63360b into master Nov 9, 2022
@mergify mergify bot deleted the markm-deprecate-kinds branch November 9, 2022 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants