Refactor codebase to use value parameters instead of pointers where appropriate
- 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
Assessment
This issue has not been assessed yet.