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

Why is there a functional pattern constructore for the installer? #381

Closed
becheran opened this issue Feb 2, 2024 · 1 comment
Closed
Labels
question Indicates that an issue, pull request, or discussion needs more information

Comments

@becheran
Copy link
Contributor

becheran commented Feb 2, 2024

func NewInstaller(opts ...installerConfig) (*Installer, error) {

I was a bit puzzeled when I first saw the function signature. I am quite familiar with the options pattern. But I have no idea how I am supposed to use this here? Was this commited by accident?

Because as a user of the library I don't have any options to set. I need to use the setter methods anyways. Such as SetLibDir(string).

Was it somehow thought to be used in the future or was it simply forgotten to be deleted?

@becheran becheran changed the title Why is there a functional pattern constructore? I don't see any way of using it? Why is there a functional pattern constructore for the installer? Feb 2, 2024
@mefellows
Copy link
Member

Yeah, it's a good question. I definitely can't remember why it was done that way, but I can see how it could be useful for an internal package. It's plausible it was initially not meant to be part of the public API.

One likely time it deviated was when I discovered that I couldn't have any CGO dependencies alongside the installer package (anything that depends on the native code by definition needs the C library to be loaded, so you can't also have code in that package that checks that it's loaded, what version it is etc.).

So a refactor may have happened which essentially made a version of this redundant. As it's in the public API now I can't remove it.

I'll close it for now, but we can re-open if there are suggestions to improve it.

@mefellows mefellows added the question Indicates that an issue, pull request, or discussion needs more information label Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Indicates that an issue, pull request, or discussion needs more information
Projects
None yet
Development

No branches or pull requests

2 participants