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

make v1 a type alias of v2, add go.mod #61

Merged
merged 3 commits into from
Nov 16, 2018
Merged

Conversation

jeanbza
Copy link
Member

@jeanbza jeanbza commented Nov 13, 2018

This commit supercedes #58 and follows #59, and is the second of two in a
series of commits to add Go module support to gax.

This commit makes v1 code type aliases of v2 code. This commit will be tagged
v1.0.1.

Reasoning for making type aliases:

Users are directly exposed to exported types in this package such as
CallOption. google-cloud-go will rely on v2 of gax, but a user's codebase is
still going to have the non-v2/ gax import paths.

We could:

  • Make google-cloud-go rely on v2 and simultaneously create a v2 of
    google-cloud-go. All existing users of google-cloud-go continue using v1 of
    gax and v1 of google-cloud-go. All folks migrating to v2 of google-cloud-go
    must go update their gax import paths to use /v2. (this is obviously
    infeasible)
  • Make all gax v1 types aliases of gax v2 types, and all v1 functions shadows
    of v2 functions. That way, google-cloud-go can rely on v2 but a user can pass
    either a v1 or v2 gax type.

This commit supercedes googleapis#58 and follows googleapis#59, and is the second of two in a
series of commits to add Go module support to gax.

This commit makes v1 code type aliases of v2 code. This commit will be tagged
v1.0.1.

Reasoning for making type aliases:

Users are directly exposed to exported types in this package such as
CallOption. google-cloud-go will rely on v2 of gax, but a user's codebase is
still going to have the non-v2/ gax import paths.

We could:

- Make google-cloud-go rely on v2 and simultaneously create a v2 of
google-cloud-go. All existing users of google-cloud-go continue using v1 of
gax and v1 of google-cloud-go. All folks migrating to v2 of google-cloud-go
must go update their gax import paths to use /v2. (this is obviously
infeasible)
- Make all gax v1 types aliases of gax v2 types, and all v1 functions shadows
of v2 functions. That way, google-cloud-go can rely on v2 but a user can pass
either a v1 or v2 gax type.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 13, 2018
v2/gax.go Show resolved Hide resolved
@jeanbza jeanbza changed the title make v1 a type alias of v2 make v1 a type alias of v2, add go.mod Nov 14, 2018
@jeanbza
Copy link
Member Author

jeanbza commented Nov 14, 2018

I've added a go.mod. One thing that was somewhat surprising was that the go.mod of the parent does not seem to require a require statement for the child. Does that meet y'alls expectations? I can add it manually but it gets removed the next time I run go mod tidy.

@pongad
Copy link
Contributor

pongad commented Nov 14, 2018

This does not at all meet my expectations 😆 I'm unsure who has the authoritative answer though :/

@jeanbza
Copy link
Member Author

jeanbza commented Nov 14, 2018

This does not at all meet my expectations 😆 I'm unsure who has the authoritative answer though :/

I've created golang/go#28801.

invoke.go Outdated Show resolved Hide resolved
@pongad
Copy link
Contributor

pongad commented Nov 15, 2018

This LGTM. Does go mod tidy delete your go.mod line though?

@jeanbza
Copy link
Member Author

jeanbza commented Nov 16, 2018

Sorry, I really should have called that out. Actually, the go.mod was generated by go mod init && go mod tidy, and subsequent tidies leave it as-is. 🎉

Copy link
Contributor

@pongad pongad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this and @bcmills for helping :)

@jeanbza
Copy link
Member Author

jeanbza commented Nov 16, 2018

+1, thanks @bcmills for the look-over.

@jeanbza jeanbza merged commit d5fa34d into googleapis:master Nov 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants