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.

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

Refs #37010 - Ensure correct yaml for netplan #10036

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

bastian-src
Copy link
Contributor

@bastian-src bastian-src commented Feb 2, 2024

The changes in #9967 introduce the save navigation operator. Unfortunately, the corresponding snippet relies on the passed arguments being true/false instead of nil.

@ekohl should we add a safety check to the preseed_netplan_generic_interface that turns nil into false or revert the save navigator change in this snippet?

Or am I overseeing something here?

@bastian-src
Copy link
Contributor Author

Katello tests seem unrelated AFAIK.

@bastian-src
Copy link
Contributor Author

@sbernhard you're right with your comments. I've changed the corresponding lines.

The introduced safe operator returns nil instead of false, but the
snippet expects true/false.
Copy link
Contributor

@sbernhard sbernhard left a comment

Choose a reason for hiding this comment

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

LGTM

@sbernhard sbernhard merged commit 8c140b0 into theforeman:develop Mar 12, 2024
40 checks passed
@ekohl
Copy link
Member

ekohl commented Mar 19, 2024

@sbernhard you can only merge with Refs if you cherry pick it to the stable branch. Otherwise you'll end up with a missing commit.

@ekohl should we add a safety check to the preseed_netplan_generic_interface that turns nil into false or revert the save navigator change in this snippet?

Apologies for missing this. I'd actually say the easier fix is to enforce a boolean in preseed_netplan_generic_interface.erb. I think #10102 is that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants