-
Notifications
You must be signed in to change notification settings - Fork 18k
path/filepath: add SkipAll #47209
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
Comments
This comment has been minimized.
This comment has been minimized.
Upon closer inspection, it does look like some replies in the other thread from the original author were deleted, so it almost looks like Ian and me were talking to ourselves. They don't affect the proposed API at all, though, since they were just talking about why the issue hadn't gotten a resolution yet. |
@cherrymui any particular reason you turned this into a proposal? I particularly wanted to avoid the red tape of waiting for the proposal review committee to pick this up :) |
The original issue is a proposal and I'm not sure why this is not. I though all new APIs are proposals? If you don't think this is a proposal, feel free to modify. Thanks. |
See this bit from the top of this thread:
I seem to recall Russ has said that the proposal review committee only needs to be looped in if a feature/API request isn't getting attention from the owners of the package. I guess there's no harm in making it a proposal, since both owners are likely busy. |
All new public API should go through the proposal process. Feel free to ping us if something urgent is not being picked up. Thanks. |
This proposal has been added to the active column of the proposals project |
Should it be StopWalk or use the same Skip prefix that SkipDir does, like SkipAll? |
/cc @robpike |
SkipAll is more consistent, StopWalk is more clear. Either is fine. |
I'd vote StopWalk |
I feel StopWalk is the better choice. SkipAll implies to me that it still scans/reads everything, but just doesn't do the callback, whereas StopWalk implies a full stop. |
On balance my vote is for Also:
|
SkipAll it is. |
Based on the discussion above, this proposal seems like a likely accept. |
No change in consensus, so accepted. 🎉 |
Change https://golang.org/cl/363814 mentions this issue: |
Fixes golang#47209 Change-Id: If75b0dd38f2c30a23517205d80c7a6683a5c921c Reviewed-on: https://go-review.googlesource.com/c/go/+/363814 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Bryan Mills <bcmills@google.com>
This is a re-opening of #43760, since the original author closed it - seemingly for their perception that there was no interest. The proposed API is small, the issue had six upvote reactions at the time of closing, and I personally think it's a good idea too, so I'm reopening here.
Since it's possible to delete GitHub comments or entire threads, I've made a waybackmachine copy as of today: https://web.archive.org/web/20210714212659/https://github.com/golang/go/issues/43760
In summary: Right now we have
filepath.SkipDir
for skipping an entire directory while walking viafilepath.Walk
, but nothing for stopping the walk entirely. It's possible to accomplish that by returning a custom error and ignoring it from thefilepath.Walk
error return, but that's a bit clunky. It would be nice to simply useStopWalk
, and haveWalk
return a nil error when it's stopped that way.The original issue was a proposal, but I don't think this is complex or controversial enough to need the process. I also think waiting to be selected for proposal review might have been what caused the confusion around the lack of interest.
cc @89z @ianlancetaylor @josharian
cc @robpike @rsc as per https://dev.golang.org/owners
The text was updated successfully, but these errors were encountered: