-
Notifications
You must be signed in to change notification settings - Fork 21
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
Enhance configuration sync and validation #36
Conversation
Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
} | ||
} | ||
|
||
export function getDefaultMemory(preset: Preset): number { |
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.
Nothing about disksize?
Did @praveenkumar keep that identical?
'OpenShift-Local.pullsecretfile': ['pull-secret-file', 'Pullsecret file path'], | ||
'OpenShift-Local.cpus': { name: 'cpus', label: 'CPUS', needDialog: true, validation: validateCpus }, | ||
'OpenShift-Local.memory': { name: 'memory', label: 'Memory', needDialog: true, validation: validateRam }, | ||
'OpenShift-Local.preset': { name: 'preset', label: 'Preset', needDialog: true, requiresRecreate: true }, |
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.
👍
'OpenShift-Local.preset': ['preset', 'Preset'], | ||
'OpenShift-Local.disksize': ['disk-size', 'Disk Size'], | ||
'OpenShift-Local.pullsecretfile': ['pull-secret-file', 'Pullsecret file path'], | ||
'OpenShift-Local.cpus': { name: 'cpus', label: 'CPUS', needDialog: true, validation: validateCpus }, |
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.
restart required
'OpenShift-Local.disksize': ['disk-size', 'Disk Size'], | ||
'OpenShift-Local.pullsecretfile': ['pull-secret-file', 'Pullsecret file path'], | ||
'OpenShift-Local.cpus': { name: 'cpus', label: 'CPUS', needDialog: true, validation: validateCpus }, | ||
'OpenShift-Local.memory': { name: 'memory', label: 'Memory', needDialog: true, validation: validateRam }, |
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.
ditto... only on restart
syncProxy(context); | ||
} | ||
|
||
async function syncProxy(context: extensionApi.ExtensionContext): Promise<void> { |
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.
Also only on restart if not mistaken.
return `Requires Memory in MiB >= ${getDefaultMemory(crcStatus.status.Preset as Preset)}`; | ||
} | ||
} | ||
|
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.
async function handleRestart(): Promise<void> {
...
buttons.unshift('Restart now');
buttons.unshift('Restart later');
...
}
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.
Please file issue to track requested enhancements.
This PR remove default and minimum values from crc-extension configuration.
Add validation of configuration property based on current preset.
Also it inform and assist user recreate instance when preset changed.
And PR add proxy setting sync, but in case of proxy settings we copy setting from podman-desktop to crc.
Resolves #32
Demo:
Screen.Recording.2023-04-04.at.15.14.40.1.mov