-
Notifications
You must be signed in to change notification settings - Fork 18
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
Intimidating code... #26
Comments
Everything reachable is part of the public API - I'm very concerned with maintaining privacy. I'd be fine with any changes that increase readability without exposing any additional API surface though! |
Would your definition of reachable include additional files? If not, we could create a This could also be done in a single file, but I'd prefer to separate out the private/public version for organization. |
Technically yes, additional files would be included. This can be avoided by bundling on prepublish, and npmignoring the unbundled files, but I'd planned to avoid a build process in this module. However, even in separate files, if |
While digging into #24, it took me a long time to understand what was going on in the codebase, and then took me even longer to understand why.
After trying to grok the code for longer than I'd like to admit, I can see that...
This code:
is essentially a different way of doing this:
And that because of attempts to maintain privacy this...
was basically the same as this...
My question is... would you be ok with:
^ I think that doing these would help to make the code much more approachable.
We could be explicit in that the internal functions/parameters are not part of the api, and will break without breaking changes.
The text was updated successfully, but these errors were encountered: