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

Export addOpt #535

Closed
sagikazarmark opened this issue Nov 3, 2022 · 2 comments
Closed

Export addOpt #535

sagikazarmark opened this issue Nov 3, 2022 · 2 comments

Comments

@sagikazarmark
Copy link

Is your feature request related to a problem? Please describe.
addOpt appears in the public API, yet it's unexported.

If someone wanted to implement a watcher outside of this library (for example prior to 1.7.0 that's the only way to make fsnotify-dependent libraries compile on unsupported platforms, see #528) and use AddWith, they can't.

Describe the solution you'd like
Export addOpt so AddWith can be implemented by watchers external to this library.

Additional context
Another reason why someone would want to implement a custom watcher if they wanted to wrap fsnotify watchers.

Or if someone created an interface to implement alternate implementations.

There are prefectly valid use cases for this.

Also, unless addOpt is experimental for some reason, exporting it is safe as it's only used internally with an unexported type.

I'll add though that generally defining an interface for this purpose is more elegant:

type AddOpt interface{
    apply(o *withOpts)
}

Having an interface that can't be implemented is more elegant IMO than an exported function that cannot be called.

@arp242
Copy link
Member

arp242 commented Nov 3, 2022

I mostly left it unexported because I'd like to keep the number of exported symbols to a minimum, which makes the overall library easier to use. And it's easy to export an unexported symbol but impossible to do the reverse. Figured I'd see if someone complains.

Ideally the need for wrapping should be addressed; it's one of those "patterns of last resort" to work around issues, although I suppose some people want to do it regardless of whether it's a good idea or needed.

@arp242
Copy link
Member

arp242 commented Apr 25, 2024

No one has reported this as a practical problem in a year and a half, so it doesn't seem to be a huge problem for folks. We can look again if practical issues arise.

@arp242 arp242 closed this as completed Apr 25, 2024
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

No branches or pull requests

2 participants