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

Move remote package to its own module #1845

Closed
1 task done
sagikazarmark opened this issue Jun 1, 2024 · 6 comments · Fixed by #1860
Closed
1 task done

Move remote package to its own module #1845

sagikazarmark opened this issue Jun 1, 2024 · 6 comments · Fixed by #1860
Labels
kind/enhancement New feature or request

Comments

@sagikazarmark
Copy link
Collaborator

Preflight Checklist

  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Problem Description

Viper has an enormous amount of dependencies (called out a lot in past, particularly in #707).

Most of those dependencies are the result of supporting different remote config providers.

22% of users who responded to the v2 feedback form claimed to use a remote provider, so dumping it completely is not an option:

Screenshot 2024-06-01 at 14 51 36

Proposed Solution

@skitt proposed (alongside with some actual proof that it works) to create a submodule out of the remote package in #707 (comment)

Although technically it's a backward incompatible change, the Go tool is clever enough to notice that an imported package is a separate module, thus able to resolve the situation without users having to do anything.

Alternatives Considered

The alternative is moving the remote package to a completely separate repo (which is probably better for the long term), but it's a hard BC break and we should aim to minimize those in a single major version.

Additional Information

No response

@sagikazarmark sagikazarmark added the kind/enhancement New feature or request label Jun 1, 2024
Repository owner deleted a comment from github-actions bot Jun 1, 2024
@sagikazarmark
Copy link
Collaborator Author

Further thinking about this: I wonder if we should just remove the remote code from the core entirely.

For example: move it to a repository under https://github.com/go-viper and just import it back for backwards compatibility.

I'd very much like to remove these features from the core eventually (ie. provide an interface that doesn't care about remote).

@skitt
Copy link
Contributor

skitt commented Jun 2, 2024

Further thinking about this: I wonder if we should just remove the remote code from the core entirely.

That would be the best long-term solution …

For example: move it to a repository under https://github.com/go-viper and just import it back for backwards compatibility.

… but importing the new module for backwards compatibility would negate the dependency benefits.

I'd very much like to remove these features from the core eventually (ie. provide an interface that doesn't care about remote).

Ultimately that sounds like a v2 feature 😉.

@sagikazarmark
Copy link
Collaborator Author

… but importing the new module for backwards compatibility would negate the dependency benefits.

I was thinking to still make the remote package a separate module AND move the code out. It might make maintenance harder, so I’m not sure yet.

Fortunately, the remote package has no exported types, so we can make that change anytime.

@skitt
Copy link
Contributor

skitt commented Jun 2, 2024

… but importing the new module for backwards compatibility would negate the dependency benefits.

I was thinking to still make the remote package a separate module AND move the code out. It might make maintenance harder, so I’m not sure yet.

Ah, right, yes, I think that would work.

@pkarakal
Copy link

pkarakal commented Jun 2, 2024

This sounds like a great proposal. Having a slimmer binary is always welcome 😝 . Moving remote package (or providers in general) to a separate repo is not needed tbh ( monorepos work great in Go)

This could also be applied to the rest of the providers (file, which could in turn also be an interface that the different types implement, env, flags). Basically the v2 can go ahead and have just a thin but pluggable core and the rest would just be separate packages and users can just import what they need.

@sagikazarmark
Copy link
Collaborator Author

Having a slimmer binary is always welcome 😝

It's not going to make binaries smaller. Go is smart enough not to compile packages that you don't import.

Moving remote package (or providers in general) to a separate repo is not needed tbh ( monorepos work great in Go)

Well, I'm not that positive about monorepos and Go. :) But the idea here is to start removing that chunk of code from the core. The downside is some additional maintenance cost when updating dependencies.

This could also be applied to the rest of the providers (file, which could in turn also be an interface that the different types implement, env, flags).

That is the plan, although most of the current implementations will probably stay in the core with the exception of a few encoding libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants