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.

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

cp: move copy_directory() to its own module #3901

Merged
merged 3 commits into from
Oct 20, 2022

Conversation

jfinkels
Copy link
Collaborator

@jfinkels jfinkels commented Sep 4, 2022

This pull request refactors copy_directory() and related helper functions into its own new module copydir.rs. This commit also adds some additional structs and helper functions to make the code easier to read and maintain. This commit does not change the behavior of the copy_directory() function, only the organization of the code.

I am proposing this change because I felt while working on pull request #3894 that it was harder than necessary for me to read and understand the code.

@sylvestre sylvestre force-pushed the cp-refactor-copy-direntry branch 2 times, most recently from 6de0201 to 88201db Compare September 10, 2022 20:21
@tertsdiepraam
Copy link
Member

Looks good! I just have the one comment above but otherwise ready to merge. Could you separate the moving around and refactoring next time, so it's easier to review?

@jfinkels
Copy link
Collaborator Author

Could you separate the moving around and refactoring next time, so it's easier to review?

Yes, absolutely.

@jfinkels
Copy link
Collaborator Author

I rebased and updated this branch so that one commit moves the copy_directory() function as-is to a new module, and the next commit breaks up the copy_directory() function into smaller helper functions.

@jfinkels jfinkels force-pushed the cp-refactor-copy-direntry branch 2 times, most recently from 4a02434 to 7b5c857 Compare September 26, 2022 01:10
@sylvestre
Copy link
Contributor

Conflicts too :/

@sylvestre
Copy link
Contributor

Conflicting again after the merge of your other PR ;)

@jfinkels jfinkels force-pushed the cp-refactor-copy-direntry branch 2 times, most recently from 940ce05 to f5c8dcc Compare October 11, 2022 02:42
@uutils uutils deleted a comment from github-actions bot Oct 13, 2022
Refactor common code into a helper method
`Options::preserve_hard_links()`. This also eliminates the need for
mutability in a local variable in two places.
Move the `copy_directory()` helper function to a new module
`copydir.rs`. This commit only changes the organization of the code,
not its behavior.
Add some additional structs and helper functions to make the code in
`copydir.rs` easier to read and maintain. This commit changes only the
organization of the code, not its function.
@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/misc/tee. tests/misc/tee is passing on 'main'. Maybe you have to rebase?

@sylvestre sylvestre merged commit 0f2067f into uutils:main Oct 20, 2022
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