Skip to content

Conversation

mprahl
Copy link
Collaborator

@mprahl mprahl commented Oct 15, 2025

Description of your changes:

This was added as a mistake as this is environment specific and not a safe default.

Checklist:

Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@mprahl mprahl force-pushed the remove-default-workspace-config branch from 7beac26 to 6454ca1 Compare October 15, 2025 17:46
@google-oss-prow google-oss-prow bot requested review from HumairAK and zazulam October 15, 2025 17:46
@mprahl mprahl force-pushed the remove-default-workspace-config branch from 6454ca1 to 064550e Compare October 15, 2025 18:01
@mprahl mprahl marked this pull request as ready for review October 15, 2025 18:40
@mprahl
Copy link
Collaborator Author

mprahl commented Oct 15, 2025

/cc @VaniHaripriya

@HumairAK
Copy link
Collaborator

/lgtm

expectError: false,
},
{
name: "workspace with nil Kubernetes config",
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reasoning behind completely removing the test cases for workspace with nil K8s config?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's because the test case workspace with size specified and default workspace now covers this scenario.

}
}

if len(pvcSpec.AccessModes) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be helpful to have a comment here explaining pvcSpec.AccessModes

This was added as a mistake as this is environment specific and not a
safe default.

Signed-off-by: mprahl <[email protected]>
@mprahl mprahl force-pushed the remove-default-workspace-config branch from 064550e to cd3b97d Compare October 20, 2025 14:40
@google-oss-prow google-oss-prow bot removed the lgtm label Oct 20, 2025
@mprahl mprahl requested a review from alyssacgoins October 20, 2025 14:40
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from humairak. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Member

@gmfrasca gmfrasca left a comment

Choose a reason for hiding this comment

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

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants