Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Do not include v1 in package path #10

Merged
merged 1 commit into from
Sep 14, 2018

Conversation

semistrict
Copy link
Contributor

The initial version of a module should not include the version
number.

See: golang/go#24301 (comment)

@odeke-em
Copy link
Contributor

odeke-em commented Sep 14, 2018

@Ramonza, thank you for the PR! I personally veto this change. While the new pragmatic advice from Go modules is for go packages and it makes sense that the first version shouldn't be tagged /v1, this repository is actually using versioned protos that MUST match against the versioned agent and also be compatible with all the clients in the various languages. I don't think the tips from gomodules apply in the case of versioned APIs which is what this repository does. That would be like expecting auto-generated Google Cloud APIs to all follow that advice and break on a field change https://github.com/googleapis/google-api-go-client/tree/master/drive.

What you linked has an exception to the rule

First, many developers will create packages that never make a breaking change once they reach v1, which is something we've encouraged from the start. We don't believe all those developers should be forced to have an explicit v1 when they may have no intention of ever releasing v2. The v1 becomes just noise. If those developers do eventually create a v2, the extra precision kicks in then, to distinguish from the default, v1.

This is code is for a large software project that we expect to change proto definitions and allow for breaking changes to happen. I'd currently rather wait until gomodules have matured and when I understand them fully to be able to make such a judgement. Many users will be using this project and I'd rather play it safe with the /vx suffix.

@semistrict
Copy link
Contributor Author

The version of the wire protocol is independent of the version of this module. You could imagine a future evolution of this module that supports multiple versions of the wire protocol without changing the module's Go API. You could also imagine the opposite scenario, where we would want to create a new major version of this module even though there hasn't been any protocol change (for example, if we decide to evolve the Exporter interface in an incompatible way).

For these reasons, I believe we should stick with the recommendation for Go modules to not include a v1 in the module path.

@semistrict
Copy link
Contributor Author

@rakyll would you like to weigh in here?

@semistrict
Copy link
Contributor Author

/cc @bogdandrutu

The initial version of a module should not include the version
number.

See: golang/go#24301 (comment)
@bogdandrutu
Copy link
Contributor

I don't think the proto version should be in this ocagent version. These are 2 different things, if you do make a breaking change on the ocagent exporter you need to go to v2 and protos may not go to v2.

Because of this I think we should use what the golang uses for versioning and not correlate this with the proto versions. Also most likely when we (if ever) add a proto/v2 we probably not completely drop the support for v1 and will have a transition point.

Anyway I agree with Ramon that the proto version should not be reflected in the go package name. I don't know what is the right thing in terms of versions for Go but we should do what the language suggest.

@semistrict semistrict merged commit 00af367 into census-ecosystem:master Sep 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants