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

replace []Issue with []*Issue and for other large structs as well #375

Merged
merged 1 commit into from
Jun 20, 2016

Conversation

gmlewis
Copy link
Collaborator

@gmlewis gmlewis commented Jun 17, 2016

Note that this is an API-breaking change but should have minimal
impact on users of this package due to the nice inference
properties of the Go programming language.

Bumped libraryVersion to 2 due to API-breaking change as
discussed in #376.

Fixes #180.

Change-Id: Ib386135e6b8f306d1f54278968c576f3ceccc4e7

@dmitshur
Copy link
Member

dmitshur commented Jun 17, 2016

Most clients will not need to change their code at all. Some will need to update it, if they have wrappers or mock implementations of these interfaces. But it's easy to do so, and it makes the API better/resolves an issue.

It is a breaking API change (even if slightly), so I'd suggest waiting to hear @willnorris's thoughts too. But I think it's worth it (at least I'm not opposed).

LGTM.

P.S. You would not be able to do this in C++ as easily, because each . in user code would have to become ->. :)

@willnorris
Copy link
Collaborator

I think more people are going to have to change their code than you think. For example, just looking at the Issues.List method, both of the two people using it would have compilation errors. Now granted, it's a very easy fix for them, but it's still a breaking change nonetheless.

That said, I'm still fine with doing it.

Note that this is an API-breaking change but should have minimal
impact on users of this package due to the nice inference
properties of the Go programming language.

Bumped `libraryVersion` to `2` due to API-breaking change as
discussed in google#376.

Fixes google#180.

Change-Id: Ib386135e6b8f306d1f54278968c576f3ceccc4e7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants