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

feat(exo): defineExoClassKitFromJSClassKit #2401

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

erights
Copy link
Contributor

@erights erights commented Aug 8, 2024

Currently just captures some noodling that @michaelfig and I did on a kit variation of defineExoClassFromJSClass. We should look an @mhofman 's variation before investing any serious time in this. @mhofman , what's the PR?

Most PRs should close a specific Issue. All PRs should at least reference one or more Issues. Edit and/or delete the following lines as appropriate (note: you don't need both refs and closes for the same one):

Closes: #XXXX
Refs: #XXXX

Description

Add a description of the changes that this PR introduces and the files that are the most critical to review.

Security Considerations

Does this change introduce new assumptions or dependencies that, if violated, could introduce security vulnerabilities? How does this PR change the boundaries between mutually-suspicious components? What new authorities are introduced by this change, perhaps by new API calls?

Scaling Considerations

Does this change require or encourage significant increase in consumption of CPU cycles, RAM, on-chain storage, message exchanges, or other scarce resources? If so, can that be prevented or mitigated?

Documentation Considerations

Give our docs folks some hints about what needs to be described to downstream users. Backwards compatibility: what happens to existing data or deployments when this code is shipped? Do we need to instruct users to do something to upgrade their saved data? If there is no upgrade path possible, how bad will that be for users?

Testing Considerations

Every PR should of course come with tests of its own functionality. What additional tests are still needed beyond those unit tests? How does this affect CI, other test automation, or the testnet?

Compatibility Considerations

Does this change break any prior usage patterns? Does this change allow usage patterns to evolve?

Upgrade Considerations

What aspects of this PR are relevant to upgrading live production systems, and how should they be addressed?

Include *BREAKING*: in the commit message with migration instructions for any breaking change.

Update NEWS.md for user-facing changes.

Delete guidance from pull request description before merge (including this!)

@mhofman
Copy link
Contributor

mhofman commented Aug 9, 2024

We should look an @mhofman 's variation before investing any serious time in this. @mhofman , what's the PR?

No PR, just a WIP commit on your previous work: ac34b4c

@mhofman
Copy link
Contributor

mhofman commented Aug 9, 2024

I have actually been wondering how we should go about defining a class kit using class syntax. For a single class, it's pretty straightforward, but for a class kit, where the constructor lives is less obvious.

@erights erights force-pushed the markm-mfig-defineExoClassKitFromJSClassKit branch 3 times, most recently from 7beca66 to 6306db2 Compare August 14, 2024 23:01
@erights erights force-pushed the markm-mfig-defineExoClassKitFromJSClassKit branch 2 times, most recently from ba6c4f5 to 4a8c50d Compare September 7, 2024 20:33
@erights erights force-pushed the markm-mfig-defineExoClassKitFromJSClassKit branch from 4a8c50d to c958191 Compare October 14, 2024 19:47
@erights erights force-pushed the markm-mfig-defineExoClassKitFromJSClassKit branch from c958191 to 94a8de2 Compare October 29, 2024 00:04
@erights erights force-pushed the markm-mfig-defineExoClassKitFromJSClassKit branch from 94a8de2 to ba2e9d7 Compare November 17, 2024 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants