-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Local/Remote Execution environment #6413
Conversation
Took more refactoring then I had hoped, but I'm happy with the end-result |
How I tested this:
|
Maybe make it a configuration option (default off, naturally). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remarkably little change to get this I see (mostly code motion), nice work. Discussing more in person.
import {ExecutionParams} from '../../types/compilation/compilation.interfaces.js'; | ||
import {BasicExecutionResult, ExecutableExecutionOptions} from '../../types/execution/execution.interfaces.js'; | ||
|
||
export interface IExecutionEnvironment { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
Breaks Heaptrack Graph in Cmake + Execution https://godbolt.org/staging/z/zz6M7TWqx |
F# execution broken https://godbolt.org/staging/z/W3nazK8oc |
The artifacts here moved from the root of the This is not super-wrong and it might be better in case of situations where you want both build and execution artifacts. So I'll add a thing to the frontend instead of changing it back. |
fixed the broken things |
Not really sure if there's anything to test here... but still, this is live :) |
Thanks! (technically it should just not break anything :) and I've tested it well enough that I'm not expecting any issues) |
First working version
This is part 1 of many for #6379
Changed in this PR:
This so that later:
Will comment out the API Endpoint before merging of course