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.

Sign up for GitHub

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

Jump to bottom

anaconda: Use erroronfail #1137

Merged
merged 2 commits into from
Jan 13, 2025
Merged

anaconda: Use erroronfail #1137

merged 2 commits into from
Jan 13, 2025

Conversation

cgwalters
Copy link
Contributor

Not doing this can silently mask errors.

xref containers/bootc#931

@cgwalters
anaconda: Use erroronfail
4a05065 
Not doing this can silently mask errors.

xref containers/bootc#931

Signed-off-by: Colin Walters <[email protected]>
mvo5
mvo5 previously approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@mvo5 mvo5 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, this looks very reasonable. There is a risk of existing things to break though (when the error in the script was inconsequential) but those are arguably are already broken :)

@cgwalters
anaconda: Don't fatally error if writing to /dev/ttyS0 fails
dec4168 
This *should* probably always work, but out of conservatism
let's continue to ignore errors.

Signed-off-by: Colin Walters <[email protected]>
@cgwalters
Copy link
Contributor Author

There's only two places that can go wrong here:

  • The switch
  • Writing the completion to /dev/ttyS0

If the switch fails then we definitely want to fatally error. The write to /dev/ttyS0 is more arguable...I pushed a commit which continues to ignore errors there.

@supakeen supakeen enabled auto-merge January 13, 2025 14:52
@supakeen
Copy link
Member

Since this only catches errors in bootc switch (debatable if the write-to-terminal can fail?) I'm ACK.

@supakeen supakeen added this pull request to the merge queue Jan 13, 2025
mvo5
mvo5 approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@mvo5 mvo5 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

Merged via the queue into osbuild:main with commit 9eaf5d0 Jan 13, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@supakeen supakeen supakeen approved these changes

@mvo5 mvo5 mvo5 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cgwalters @supakeen @mvo5