search

RedHatInsights / yggdrasil

GNU General Public License v3.0
21 stars 37 forks source link

Rename module to include a v1 suffix #250

Closed subpop closed 1 month ago

subpop commented 2 months ago

Since the API of the yggdrasil-0.2 branch is frozen, adding an API suffix shouldn't be harmful, but would require clients to update their import paths when updating to newer versions of the yggdrasil-0.2 project.

jirihnidek commented 2 months ago

Not sure about this. I don't have any experience with such change. Why is it necessary or why is it needed?

subpop commented 2 months ago

Not sure about this. I don't have any experience with such change. Why is it necessary or why is it needed?

This is the approach recommended by the Go community for handling API compatibility. Many projects follow a similar pattern of appending a version into the module path.

Specifically for our case here, I'm proposing this change to support embedding yggdrasil-0.2.x into the foreman worker so that it can support both the gRPC (API v1) and D-Bus (API v2) versions of yggdrasil in a single program. In order to accomplish this currently, I have vendored an entire copy of yggdrasil-0.2.x into their worker, but a far cleaner approach is to use a submodule that pins yggdrasil to a certain commit. In order to be able to import both versions of yggdrasil into a single project, they need to have unique import paths. Since the gRPC version of yggdrasil is essentially API-frozen, moving it to an import path of ".../v1" allows for projects to update to yggdrasil and still carry v1 as a fallback case (if they wanted to).

This won't actually break any projects until they update the version of yggdrasil they're importing, so, for example, we will have to make some changes to rhc-0.2.x if and when we make a new release of it. Or the 0.1.x branch of yggdrasil-worker-package-manager, when it wants to update the version of yggdrasil it pins in go.mod, we'll have to update the import paths.