-
Notifications
You must be signed in to change notification settings - Fork 312
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
fix: Walkfunc should check the err #460
fix: Walkfunc should check the err #460
Conversation
fix #459 |
Codecov Report
@@ Coverage Diff @@
## main #460 +/- ##
==========================================
- Coverage 67.97% 67.95% -0.03%
==========================================
Files 155 155
Lines 16576 16579 +3
==========================================
- Hits 11268 11266 -2
- Misses 4474 4478 +4
- Partials 834 835 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
18faf92
to
e4de897
Compare
@j4ckstraw Could you please add a unit test for this fix? |
Let me try |
@j4ckstraw any progress with this PR? |
sorry, I am busy recently. BYW, Is it really need a test? |
Actually no, but we would love to see the code coverage getting higher. We can still merge this PR if you want, it's your call, please let me know. |
We will appreciate it if you can make a testable patch and help us avoid regression. |
I have a problem when try to write a test for this function. I know that if the path which stat call is not found, the Walk function will return error. Now I can reproduce only in race condition, I have no idea to write a test for it. help! |
Hi @j4ckstraw, please rebase to the latest code. Codecov won't be a problem anymore, we can merge this PR then. |
Signed-off-by: j4ckstraw <j4ckstraw@foxmail.com>
e4de897
to
420dc65
Compare
@j4ckstraw have you joined our DingTalk or WeChat group? I would like to discuss further cooperation with you~ |
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.
/lgtm
Not yet, It would be my pleasure. |
Nice! You can find our DingTalk QR Code in README |
|
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hormes The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: j4ckstraw j4ckstraw@foxmail.com
Ⅰ. Describe what this PR does
Ⅱ. Does this pull request fix one issue?
fix #459
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews
V. Checklist
make test