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

Overhaul swingset configuration #1381

Merged
merged 2 commits into from
Aug 6, 2020
Merged

Overhaul swingset configuration #1381

merged 2 commits into from
Aug 6, 2020

Conversation

FUDCo
Copy link
Contributor

@FUDCo FUDCo commented Aug 6, 2020

These changes implement the new swingset configuration model per #1331. A writeup of the deltas is in the file APIChange.md (which should probably be rehomed or renamed or incorporated into some other file, but I wanted to get these changes documented without delaying for the time it would take me to get clear on the change notes publication flow).

@FUDCo FUDCo requested a review from warner August 6, 2020 02:23
@FUDCo FUDCo added enhancement New feature or request SwingSet package: SwingSet swingset-runner package: swingset-runner labels Aug 6, 2020
@warner
Copy link
Member

warner commented Aug 6, 2020

It looks like some more changes landed on trunk after you branched off (but not mine, so I think they won't conflict). I'm going to experiment with github's "Update branch" button and see what it does. I think we'll want to rebase this whole thing before landing, to avoid the little rats nest of branches that this button will probably create.

@warner
Copy link
Member

warner commented Aug 6, 2020

Yep, that button created a merge commit between the branch you pushed and current trunk, which is more messy than we'd want to land. But a git rebase master cleans it up just fine, so let's remember to either do that manually, or use the Merge button with the "Rebase" option.

@FUDCo FUDCo force-pushed the refactor-swingset-config branch 2 times, most recently from 4cd6ad7 to 06151c6 Compare August 6, 2020 07:32
Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good. I"m still nervous about YAGNI, but I like everything here. A few minor changes to make, feel free to land once those are done.

packages/SwingSet/src/controller.js Outdated Show resolved Hide resolved
packages/SwingSet/src/controller.js Outdated Show resolved Hide resolved
packages/SwingSet/src/controller.js Show resolved Hide resolved
packages/SwingSet/src/kernel/kernel.js Outdated Show resolved Hide resolved
packages/SwingSet/src/kernel/kernel.js Outdated Show resolved Hide resolved
packages/SwingSet/src/kernel/kernel.js Show resolved Hide resolved
packages/SwingSet/src/kernel/kernel.js Outdated Show resolved Hide resolved
packages/SwingSet/test/test-transcript.js Outdated Show resolved Hide resolved
@FUDCo FUDCo force-pushed the refactor-swingset-config branch from 06151c6 to 6a4dee8 Compare August 6, 2020 19:10
@FUDCo FUDCo merged commit 8748674 into master Aug 6, 2020
@FUDCo FUDCo deleted the refactor-swingset-config branch August 6, 2020 20:48
Chris-Hibbert added a commit that referenced this pull request Dec 1, 2022
Modulo one issue that seems related to loopback (#1381), all tests
pass, but there's a little more work to do.

A few pieces of Zoe don't use Far classes. This should be a simple
update.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet swingset-runner package: swingset-runner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants