-
Notifications
You must be signed in to change notification settings - Fork 155
Etc merge fix #1649
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
Etc merge fix #1649
Conversation
We weren't checking if the deleted path is a file or a directory and were calling `remove_file` unconditionally. Update to check for file/directory first Signed-off-by: Pragyan Poudyal <[email protected]>
crates/lib/src/cli.rs
Outdated
| print_diff(&diff, &mut std::io::stdout()); | ||
|
|
||
| if merge { | ||
| let n = n.ok_or(anyhow::anyhow!("Failed to get dirtree for new etc"))?; |
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.
I think clippy will want ok_or_else to avoid constructing the error always (here and elsewhere)
crates/etc-merge/src/lib.rs
Outdated
| Err(e) => Err(e)?, | ||
| let stat = new_etc_fd.metadata_optional(&removed)?; | ||
|
|
||
| let stat = match stat { |
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.
Also a let-else candidate
Signed-off-by: Pragyan Poudyal <[email protected]>
fd3b99a to
1b39153
Compare
| traverse_etc(&pristine_etc, ¤t_etc, &new_etc)?; | ||
| traverse_etc(&pristine_etc, ¤t_etc, Some(&new_etc))?; | ||
|
|
||
| let new_files = new_files.ok_or(anyhow::anyhow!("Failed to get dirtree for new etc"))?; |
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.
NB another ok_or_else
No description provided.