google / google/go-github

Refactor codebase to use value parameters instead of pointers where appropriate

Open
#3,644 24 comments 4 reactions 3 assignees Claimed by @ManavSharma142 View on GitHub
Breaking API Change enhancement good first issue
Dominant language
Go
Stars
11.3k
Forks
2.5k
Avg merge
1d 22h
Merged PRs (30d)
49

Description

Many methods in the codebase, such as:

```go

func (s *RepositoriesService) CreateRelease(ctx context.Context, owner, repo string, release *RepositoryRelease) (*RepositoryRelease, *Response, error)
```

use pointer parameters (e.g. `*RepositoryRelease`) even when the function doesn't mutate the input. This appears to be a result of convention or copy-paste, not a performance-driven choice.

For instance, in `CreateRelease`, the `RepositoryRelease` struct is relatively large (~15+ fields), but most of its fields are pointers themselves. As such, passing it by value would have negligible performance impact in typical API usage. Moreover, value semantics would improve clarity, especially for required arguments that aren't modified.

To improve API design consistency and safety, we should:

- Audit method signatures that accept pointer structs as input
- Change them to accept values when:

- The input is required (not optional)

- The function does not modify the input

- Optionally introduce dedicated input structs for specific operations (e.g. `CreateRepositoryRelease` with only the fields needed for creation)

Note: This would be a breaking change and should be handled carefully, possibly across multiple PRs.

_Originally posted by @gmlewis in https://github.com/google/go-github/pull/3636#discussion_r2228429223_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.