dracut/30ignition/module-setup.sh: include groupmod binary in initramfs #2190
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly adds the groupmod binary to the initramfs, which is necessary for related changes. The PR also includes several stylistic improvements, such as code formatting and indentation fixes. I have one suggestion to further improve the readability and robustness of the install_ignition_unit function by simplifying its argument parsing.
| local unit="$1" | ||
| shift | ||
| local target="${1:-ignition-complete.target}" | ||
| shift | ||
| local instantiated="${1:-$unit}" | ||
| shift |
There was a problem hiding this comment.
While you're refactoring this for style, you can simplify the argument parsing. Using positional parameters directly ($1, $2, etc.) is clearer and less prone to errors than using shift multiple times, especially if set -e were active and the function was called with fewer arguments than expected.
| local unit="$1" | |
| shift | |
| local target="${1:-ignition-complete.target}" | |
| shift | |
| local instantiated="${1:-$unit}" | |
| shift | |
| local unit="$1" | |
| local target="${2:-ignition-complete.target}" | |
| local instantiated="${3:-$unit}" |
|
Hi, @prestist. Is there an approximate timeline as to when this PR can be merged? Thank you. |
|
@onurhanak thank you for working on this! sorry I had been busy on other tasks and this slipped my radar. I will take a look tomorrow. |
|
Hi @prestist , is there any progress on this? Is there anything I can do to speed things up? |
3 participants
Resolves #2186 . Adds groupmod binary to initramfs for the changes made in pull request #2158 to work.