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.

Sign up for GitHub

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

Jump to bottom

Inbox Windows Sandbox Environments extension #2752

Merged
merged 32 commits into from
May 23, 2024
Merged

Inbox Windows Sandbox Environments extension #2752

merged 32 commits into from
May 23, 2024

Conversation

kvega005
Copy link
Contributor

Summary of the pull request

Add Windows Sandbox Extension for Dev Home.

krschau
krschau reviewed Apr 30, 2024
View reviewed changes
@kvega005 kvega005 marked this pull request as ready for review May 3, 2024 02:18
@nieubank nieubank changed the title User/kevinve/wsb extension Inbox Windows Sandbox Environments extension May 3, 2024
@nieubank nieubank added this to the Dev Home v0.14 milestone May 3, 2024
@dhoehna
Copy link
Contributor

dhoehna commented May 3, 2024

@kvega005 can you add pictures to the PR to show us where the images are being used?

@kvega005 kvega005 linked an issue May 3, 2024 that may be closed by this pull request
Add An Extension for Windows Sandbox #2801
Closed
@kvega005 kvega005 closed this May 3, 2024
@kvega005 kvega005 reopened this May 3, 2024
@krschau krschau removed this from the Dev Home v0.14 milestone May 3, 2024
bbonaby
bbonaby reviewed May 3, 2024
View reviewed changes
extensions/WindowsSandboxExtension/Providers/WindowsSandboxComputeSystem.cs Outdated
Comment on lines 78 to 80
ComputeSystemProperty.Create(ComputeSystemPropertyKind.AssignedMemorySizeInBytes, 4294967296),
ComputeSystemProperty.Create(ComputeSystemPropertyKind.StorageSizeInBytes, 85899345920),
ComputeSystemProperty.Create(ComputeSystemPropertyKind.UptimeIn100ns, 100),
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: if you don't provide us with these we won't display them, so its ok to not send these if there is no reliable way to get this data specifically for sandbox

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am just providing the defaults for Sandbox. Right now, they are hard coded values, but when we implement the create compute system path, I want users to be able to select different options for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see, then I wonder if its a good idea to make these const variable with a name e.g FourGbInBytes etc so we know at a glance what the value is.

bbonaby
bbonaby reviewed May 3, 2024
View reviewed changes
extensions/WindowsSandboxExtension/Providers/WindowsSandboxComputeSystem.cs Outdated
Comment on lines 78 to 80
ComputeSystemProperty.Create(ComputeSystemPropertyKind.AssignedMemorySizeInBytes, 4294967296),
ComputeSystemProperty.Create(ComputeSystemPropertyKind.StorageSizeInBytes, 85899345920),
ComputeSystemProperty.Create(ComputeSystemPropertyKind.UptimeIn100ns, 100),
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see, then I wonder if its a good idea to make these const variable with a name e.g FourGbInBytes etc so we know at a glance what the value is.

@bbonaby
Copy link
Contributor

bbonaby commented May 3, 2024

I think those last 2 comments I had should be it. Thanks for doing this Kevin!

@kvega005 kvega005 merged commit 34b762e into microsoft:main May 23, 2024
4 checks passed
@bbonaby bbonaby requested a review from pehammer May 31, 2024 19:13
kvega005 pushed a commit that referenced this pull request Jun 11, 2024
Revert "Inbox Windows Sandbox Environments extension (#2752)"
8b1ad67 
This reverts commit 34b762e.
EricJohnson327 pushed a commit that referenced this pull request Jun 11, 2024
@kvega005
User/kevinve/remove sandbox extension (#3196)
068fb8c 
* Revert "Inbox Windows Sandbox Environments extension (#2752)"

This reverts commit 34b762e.

* Remove Sandbox project from solution

---------

Co-authored-by: kevinve <kevinve@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vineeththomasalex vineeththomasalex vineeththomasalex left review comments

@krschau krschau krschau left review comments

@bbonaby bbonaby bbonaby left review comments

@dhoehna dhoehna dhoehna approved these changes

@nieubank nieubank nieubank approved these changes

@kanismohammed kanismohammed Awaiting requested review from kanismohammed

@EricJohnson327 EricJohnson327 Awaiting requested review from EricJohnson327

@pehammer pehammer Awaiting requested review from pehammer

Assignees
No one assigned
Labels
merge-conflicts
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add An Extension for Windows Sandbox
6 participants
@kvega005 @dhoehna @bbonaby @nieubank @vineeththomasalex @krschau