Go-github: RepositoriesService.ReplaceAllTopics has *Topics parameter, it'd be better made a value.

[dmitshur]

I just noticed the signature of RepositoriesService.ReplaceAllTopics has a *Topic parameter.

The parameter is not optional, it's mandatory. See https://developer.github.com/v3/repos/#replace-all-topics-for-a-repository:

There's no value in making it a pointer rather than value, only harm, because it makes it possible to pass nil value.

I suggest we making a breaking API change to improve the API of go-github and change the signature of the method to:

func (s *RepositoriesService) ReplaceAllTopics(ctx context.Context, owner, repo string,
    topics Topics) (*Topics, *Response, error)

This is a new method, still under a preview API, so a breaking change is acceptable IMO, as long as it leads to an improve go-github API.

/cc @gmlewis

[dmitshur]

Actually, better yet, we can also consider making it a topics []string instead:

func (s *RepositoriesService) ReplaceAllTopics(ctx context.Context, owner, repo string,
    topics []string) (*Topics, *Response, error)

[dmitshur]

If we go with topics []string, then #771 becomes no longer neccessary, since Topics won't necessarily be sent. It might instead be:

if topics == nil {
    topics = []string{}
}

t := struct {
    Names []string `json:"names"`
}{
    Names: topics,
}
resp, err := s.client.Do(ctx, req, t)

[jpbelanger-mtl]

Just let me know what you decide. Breaking API change is not a deal breaker on this, still a very simple preview feature from github.

[gmlewis]

Yes, I very much like changing the API to use topics []string in the parameter list, as suggested by @shurcooL above.

Thank you, @shurcooL and @jpbelanger-mtl!

[sahildua2305]

@shurcooL I think this issue can be closed. Is there anything left to be fixed after #771?

[dmitshur]

Thanks for noting that @sahildua2305, you're right.

Closing, this is resolved via #771.