Skip to content

install: fix simplify conditional expressions and improve error handling#11274

Closed
mattsu2020 wants to merge 1 commit intouutils:mainfrom
mattsu2020:install
Closed

install: fix simplify conditional expressions and improve error handling#11274
mattsu2020 wants to merge 1 commit intouutils:mainfrom
mattsu2020:install

Conversation

@mattsu2020
Copy link
Contributor

Refactor several conditional blocks to use let-else statements for cleaner code flow. Simplify error handling in file removal operations by combining conditions. Update backup file handling to use more concise conditional logic. These changes improve code readability while maintaining the same functionality.

Refactor several conditional blocks to use let-else statements for cleaner code flow. Simplify error handling in file removal operations by combining conditions. Update backup file handling to use more concise conditional logic. These changes improve code readability while maintaining the same functionality.
@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/install/install-C-root. tests/install/install-C-root is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tty/tty-eof (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/rm/many-dir-entries-vs-OOM is now being skipped but was previously passing.
Congrats! The gnu test tests/unexpand/bounded-memory is now passing!

@xtqqczze
Copy link
Contributor

It looks like these changes simply fix a few instances of clippy::collapsible_if.

if !b.unprivileged && group_id != to_meta.gid() {
return true;
}
if let Some(group_id) = b.group_id
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a change in behavior

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

Yes, you are right, it's a change in behavior. The current implementation never returns true if group_id == to_meta.gid() whereas the proposed change might return true in such a case (depending on what needs_copy_for_ownership(to, &to_meta) returns).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can't make any assumptions of correctness with AI generated PRs; we have to verify everything. In particular changes that "simplify conditional expressions" are high-risk.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cakebaker In this case there actually are conditions that can be made much more clear, I've submitted #11280.

@xtqqczze
Copy link
Contributor

xtqqczze commented Mar 10, 2026

It looks like these changes simply fix a few instances of clippy::collapsible_if.

Since the remaining changes are purely clippy::collapsible_if fixes, I’ve submitted #11285 with a description of how the fixes were applied, without any AI involvement.

@mattsu2020 mattsu2020 deleted the install branch March 11, 2026 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants