-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
cp: disabled verbose output if file has been skipped #7347
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: disabled verbose output if file has been skipped #7347
Conversation
|
Could you please add a test to make sure we don't regress? Thanks |
|
GNU testsuite comparison: |
1 similar comment
|
GNU testsuite comparison: |
|
I know the compiler is smart enough to understand that the |
|
I'm not a fan of using a boolean, as it often makes the code harder to read. Maybe it helps to think about what the function does:
Thus, we have three potential outcomes: two success cases and one error case. My suggestion is to explicitly express the success cases as an enum, something like: enum SomeName {
Copied,
Skipped,
} |
I also like self explaining variables and return types. And I agree that: enum Skipped {
No,
Yes
} is a bad name. I reused the What do you think about: enum PerformedAction {
Copied,
Skipped
} |
|
Sounds good to me 👍 |
|
You can fixup the rename commit ? |
1297a76 to
f6c70f3
Compare
|
I mean your third commit should be stashed in the first or second one. |
f6c70f3 to
63fe4fb
Compare
|
GNU testsuite comparison: |
|
LGTM 👍 |
|
@BigPapa314 thanks for your PR! |
Fixes #7296