-
Notifications
You must be signed in to change notification settings - Fork 32
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
Fix vlan id should be integer type #1099
Conversation
Signed-off-by: andy.lee <andy.lee@suse.com>
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.
LGTM. Thanks for the quick fix.
Why is this necessary? The VLAN ID parameter is mandatory and empty isn't accepted anyways? |
It's straighforward to allow user clean up any input in a form. But original logic keep vlan ID to 1 which is not intuitive. We have final validation on L116, so I think it's okay to allow clean up the vlan ID input. |
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.
test and work for me, thanks.
@mergify backport release-harvester-v1.4 |
✅ Backports have been created
|
@mergify backport release-harvester-v1.3 |
@mergify backport release-harvester-v1.2 |
✅ Backports have been created
|
✅ Backports have been created
|
Summary
PR Checklist
Related Issue #
vlan
property now in UI as string, validator expectsint
harvester#6309Occurred changes and/or fixed issues
Technical notes summary
Note. this PR should backport to v1.4 / v1.3 / v1.2
Areas which could experience regressions
Screenshot/Video
fixed.mov