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

refactor(SwingSet): unify vat-worker filenames #2204

Closed
wants to merge 1 commit into from

Conversation

dckc
Copy link
Member

@dckc dckc commented Jan 15, 2021

fixes #2202

Makes me glad I invested in learning vs-code!

@dckc dckc requested review from warner and kriskowal January 15, 2021 20:41
@dckc dckc force-pushed the 2202-vat-worker-names branch from 81823ec to 05834ca Compare January 16, 2021 18:21
@dckc dckc force-pushed the 2202-vat-worker-names branch from 05834ca to a5014c0 Compare January 16, 2021 21:31
@dckc
Copy link
Member Author

dckc commented Jan 17, 2021

#2158 shows this is incomplete.

@dckc
Copy link
Member Author

dckc commented Jan 18, 2021

the filenames we're unifying also show up in a couple require.resolve calls in controller.js:

fbbd535

cc @warner

warner added a commit that referenced this pull request Jan 19, 2021
@warner
Copy link
Member

warner commented Jan 19, 2021

Once #2217 lands, all workers should be getting exercised.

the filenames we're unifying also show up in a couple require.resolve calls in controller.js:

Yeah, controller.js is run inside the Start Compartment, so that's where we need to define the functions that create a new Node.js thread (new Worker(supercode)) and/or spawn a child process. It's also where we do the path algebra to locate the supervisor code that must be loaded into those new containers.

I'd suggest renaming those two functions (makeNodeWorker, startSubprocessWorker, startSubprocessWorkerXS) to match the pattern we're using in the other files, and of course update those filenames to match your file renames in src/kernel/vatManager/.

@dckc dckc mentioned this pull request Jan 21, 2021
7 tasks
@dckc
Copy link
Member Author

dckc commented Jan 21, 2021

I rolled this into #2225

@dckc dckc closed this Jan 21, 2021
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.

unify vat-worker filenames
2 participants