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: move eval into a separate primer-eval package #1080

Closed
wants to merge 1 commit into from

Conversation

dhess
Copy link
Member

@dhess dhess commented Jun 20, 2023

Still very much a WIP. The tests need fixing up, and primer-service will need to combine the sessions-and-actions API with the factored-out eval API. None of that work has been done yet, but primer and primer-eval do compile, at least.

I'm open to suggestions/techniques for how best to combine the two APIs.

(One could also imagine a further evolution where we move actions & type checking to the frontend, as well, leaving only the sessions API, i.e., program store, for primer-service.)

@dhess dhess added the WIP A work in progress, not ready for merging label Jun 20, 2023
@dhess dhess marked this pull request as draft June 20, 2023 00:21
@dhess
Copy link
Member Author

dhess commented Jun 22, 2023

After chatting with @brprice today, I think this is probably the wrong approach, or at least, the wrong order in which to do things. The tentative plan now is:

  1. "chop off" the Primer.API module and its dependencies into a new primer-api package.
  2. Split out the eval-related bits of the remaining core package into primer-eval, similar to what I've done here.
  3. Optionally split out the type checking & editing bits of the core package into, e.g., primer-typecheck, leaving primer-core as the bits that both primer-eval and primer-typecheck depend on.

Then we'd have a pretty good story for compiling primer-eval to Wasm and running that in the browser, with a minimal footprint of code.

@dhess dhess closed this Jun 22, 2023
@dhess dhess deleted the dhess/primer-eval branch June 22, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP A work in progress, not ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant