Skip to content

Add create pod validation for name/port mapping#1482

Merged
martinpitt merged 1 commit into
cockpit-project:mainfrom
jelly:create-pod-port-validation
Nov 13, 2023
Merged

Add create pod validation for name/port mapping#1482
martinpitt merged 1 commit into
cockpit-project:mainfrom
jelly:create-pod-port-validation

Conversation

@jelly
Copy link
Copy Markdown
Member

@jelly jelly commented Nov 10, 2023

Recently the Image Create modal implemented validation for Volume/Port mapping. Re-use the same changes in Podman except for Volumes as there can be some improvements done there.

I'm happy to integrate the volume validation after #1481

Recently the Image Create modal implemented validation for Volume/Port
mapping. Re-use the same changes in Podman except for Volumes as there
can be some improvements done there.
@jelly jelly requested a review from martinpitt November 10, 2023 11:22
Comment thread src/PodCreateModal.jsx
const dynamicListOnValidationChange = (value, key) => {
setValidationFailed(prevState => {
prevState[key] = value;
if (prevState[key].every(a => a === undefined))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

Comment thread src/PodCreateModal.jsx
const podNameValidation = validatePodName(podName);

if (podNameValidation)
newValidationFailed.containerName = podNameValidation;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

Copy link
Copy Markdown
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thank you!

@martinpitt martinpitt merged commit 79f1001 into cockpit-project:main Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants