-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Draft public sdk api for wsla #13827
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
base: feature/wsl-for-apps
Are you sure you want to change the base?
Conversation
| set(SOURCES wsla.cpp) | ||
| set(HEADERS wsla.h) | ||
|
|
||
| add_library(wslasdk SHARED ${SOURCES} ${HEADERS} wslasdk.def) |
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.
I was originally of having the methods inside the wslaclient target, since it's essentially going to be the same thing conceptually.
I don't mind renaming it though, but if we do that let's delete the old target
|
|
||
| // Container Process | ||
|
|
||
| STDAPI WslaCreateContainerProcess(_In_ WslaRuntimeContainer container, _In_ const WSLA_CONTAINER_PROCESS_OPTIONS* options, _Out_ WSLA_CONTAINER_PROCESS* process); |
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.
We might want to follow the COM logic a bit closer to make the integration easier.
Something like:
WslaGetContainerInitProcess(WslaRuntimeContainer container, WSLA_CONTAINER_PROCESS* process);
might be easier to implement
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.
This one was the "nerdctl exec". I think you mean splitting CreateNewContainer into 2 methods? That works for me too.
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed