-
Notifications
You must be signed in to change notification settings - Fork 147
lib/src/reboot: use valid systemd-run arguments #1417
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
Conversation
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.
Code Review
This pull request correctly fixes an invalid argument passed to systemd-run by replacing --message with the valid --description argument. The change is accurate and addresses the issue described. I've added one suggestion to improve code clarity and maintainability by separating the command-line flag and its value, which will also make it easier to manage the description string in the future.
|
Ugh having that previous PR get through CI is a very serious regression we need to root cause. What's going on here is that Also in this we should separate the arguments via |
cgwalters
left a comment
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.
See review comments
Yes we must fix this immediately |
|
Pushed a new commit passing it in as so: This successfully rebooted my machine. Starting to look at tests, but this can probably be merged before then. |
Signed-off-by: Robert Sturla <[email protected]>
cgwalters
left a comment
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.
Looks reasonable to me as is. I'm going to pull this PR and test it out myself now too.
|
Getting sidetracked by a testing framework, still looking at that |
Closes #1416
Follow up to #1398
Switches to use--descriptioninstead of--messagesince Gemini's suggestion in the previous PR wasn't correct.The current
--messageargument is not passed in correctly, causing reboots to never be invoked.This PR appends the arguments to the reboot command correctly, which should result in successful restarts.
Would this bug warrant a new patch release? Concerned that if users are relying on the auto-update-reboot feature, their machines will not be updated until they manually reboot.