diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1c3d86f7a4d..92ded03a4b6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,10 +18,10 @@ again. ## Reporting issues Bugs, feature requests, and development-related questions should be directed to -our [GitHub issue tracker](https://github.com/google/go-github/issues). If +our [GitHub issue tracker](https://github.com/google/go-github/issues). If reporting a bug, please try and provide as much context as possible such as your operating system, Go version, and anything else that might be relevant to -the bug. For feature requests, please explain what you're trying to do, and +the bug. For feature requests, please explain what you're trying to do, and how the requested feature would help you do that. Security related bugs can either be reported in the issue tracker, or if they @@ -151,7 +151,13 @@ tips (which are frequently ignored by AI-driven PRs): * When possible, try to make smaller, focused PRs (which are easier to review and easier for others to understand). -## Code Comments +## Code Guidelines + +This section documents common code patterns and conventions used throughout +the go-github repository. Following these guidelines helps maintain consistency +and makes the codebase easier to understand and maintain. + +### Code Comments Every exported method and type needs to have code comments that follow [Go Doc Comments][]. A typical method's comments will look like this: @@ -163,9 +169,9 @@ Every exported method and type needs to have code comments that follow // //meta:operation GET /repos/{owner}/{repo} func (s *RepositoriesService) Get(ctx context.Context, owner, repo string) (*Repository, *Response, error) { - u := fmt.Sprintf("repos/%v/%v", owner, repo) - req, err := s.client.NewRequest(ctx, "GET", u, nil) - ... + u := fmt.Sprintf("repos/%v/%v", owner, repo) + req, err := s.client.NewRequest(ctx, "GET", u, nil) + ... } ``` And the returned type `Repository` will have comments like this: @@ -173,10 +179,10 @@ And the returned type `Repository` will have comments like this: ```go // Repository represents a GitHub repository. type Repository struct { - ID *int64 `json:"id,omitempty"` - NodeID *string `json:"node_id,omitempty"` - Owner *User `json:"owner,omitempty"` - ... + ID *int64 `json:"id,omitempty"` + NodeID *string `json:"node_id,omitempty"` + Owner *User `json:"owner,omitempty"` + ... } ``` @@ -199,6 +205,372 @@ description. [Go Doc Comments]: https://go.dev/doc/comment +### Naming Conventions + +#### File Names + +Files are organized by service and API endpoint, following the pattern +`{service}_{api}.go`. For example: +- `repos_contents.go` - Repository contents API methods +- `users_ssh_signing_keys.go` - User SSH signing keys API methods +- `orgs_rules.go` - Organization rules API methods + +Test files follow the pattern `{service}_{api}_test.go`. + +These services map directly to how the [GitHub API documentation][] is +organized, so use that as your guide for where to put new methods. + +[GitHub API documentation]: https://docs.github.com/en/rest + +#### Receiver Names + +Service method receivers consistently use the single letter `s`: + +```go +func (s *RepositoriesService) Get(ctx context.Context, owner, repo string) (*Repository, *Response, error) { + // ... +} +``` + +#### Request Option Types + +Request option types (for query parameters) are named after the method they +modify, with the suffix `Options`: + +```go +type RepositoryListOptions struct { + Type string `url:"type,omitempty"` + Sort string `url:"sort,omitempty"` + Direction string `url:"direction,omitempty"` + ListOptions +} +``` + +#### Request Body Types + +Request body types (for POST/PUT/PATCH requests) use either `Options` or +`Request` as a suffix. Both patterns exist in the codebase: + +```go +// With Options suffix: +type RepositoryContentFileOptions struct { + Message *string `json:"message,omitempty"` + Content []byte `json:"content"` + SHA *string `json:"sha,omitempty"` + Branch *string `json:"branch,omitempty"` + Author *CommitAuthor `json:"author,omitempty"` + Committer *CommitAuthor `json:"committer,omitempty"` +} + +// With Request suffix: +type CreateHostedRunnerRequest struct { + Name string `json:"name"` + RunnerGroupId int64 `json:"runner_group_id"` + EnableStaticIP *bool `json:"enable_static_ip,omitempty"` + Image string `json:"image"` + VMSize string `json:"vm_size"` +} +``` + +When adding new request body types, prefer the `Request` suffix for +create/update operations and `Options` for other operations, but follow +the existing patterns in the relevant service file. + +#### Response Types + +Response types are named after the resource they represent, typically without +any suffix: + +```go +type Repository struct { + ID *int64 `json:"id,omitempty"` + Name *string `json:"name,omitempty"` + FullName *string `json:"full_name,omitempty"` + Description *string `json:"description,omitempty"` + // ... +} +``` + +For response body fields, use pointer types with `omitempty` for optional +fields and non-pointer types for required fields. Note that many existing +response types use pointer types for all fields; when adding new fields, +prefer non-pointer types where zero values are acceptable or where the +distinction between "zero" and "unset" is not needed. + +#### Method and Variable Naming + +Methods use descriptive names that clearly indicate their action: +- `Get` - Retrieve a single resource +- `List` - Retrieve multiple resources (supports pagination) +- `Create` - Create a new resource +- `Update` - Update an existing resource +- `Delete` - Delete a resource + +Common local variable names: +- `ctx` - Context +- `u` - URL string +- `req` - HTTP request +- `resp` - HTTP response +- `result` - Result from API call +- `err` - Error +- `opts` - Options parameter + +#### Common Variable Names + +These variable names are used consistently throughout the codebase: +- `owner` - Repository owner (username or organization) +- `repo` - Repository name +- `org` - Organization name +- `user` - Username +- `team` - Team name or slug +- `project` - Project name or number + +#### Scoped Methods + +Methods are defined on service types that already define their scope, so +the method name does not need to repeat the scope. For example: + +```go +// On OrganizationsService, the scope is already "organization": +func (s *OrganizationsService) ListMembers(ctx context.Context, org string, opts *OrganizationListMembersOptions) ([]*User, *Response, error) + +// On EnterpriseService, the scope is already "enterprise": +func (s *EnterpriseService) GetLicenseInfo(ctx context.Context) (*LicenseInfo, *Response, error) +``` + +#### Value Parameters + +Prefer value types over pointer types for parameters where the distinction +between "zero" and "unset" is not needed, or where the type is small and +cheap to copy (e.g., `string`, `int`, `int64`, `bool`). Use pointer types +when you need to distinguish between an unset value and a zero value. + +#### Common Structs + +These common structs are used throughout the codebase: +- `ListOptions` - For offset-based pagination (page/per_page) +- `ListCursorOptions` - For cursor-based pagination +- `UploadOptions` - For file uploads +- `Response` - Wraps the HTTP response + +### JSON Tags + +#### Request Bodies + +When defining structs that represent a request body (sent via POST/PUT/PATCH): + +1. Required fields should be non-pointer types without `omitempty`: + +```go +type SecretScanningAlertUpdateOptions struct { + State string `json:"state"` // Required field + // ... +} +``` + +2. Optional fields should be pointer types with `omitempty`: + +```go +type SecretScanningAlertUpdateOptions struct { + State string `json:"state"` + Resolution *string `json:"resolution,omitempty"` + ResolutionComment *string `json:"resolution_comment,omitempty"` +} +``` + +3. Use `omitzero` for structs and `time.Time` where you want to omit + empty values (not just nil). For slices and maps, `omitzero` has the + opposite behavior: it keeps empty (non-nil) values and only omits nil + values. See `RepositoryRuleset` for examples of `omitzero` usage with + both slices and structs. + +4. For boolean fields in request bodies, use non-pointer `bool` (without + `omitempty` or `omitzero`) for required fields, and `*bool` with + `omitempty` for optional fields. In cases where you need to distinguish + between `false` and "not set", use `*bool` with `omitzero`. + +#### Response Bodies + +When defining structs that represent a response body (returned from API calls): + +1. Most fields in response types are pointers with `omitempty`, since the + GitHub API may omit fields from responses. + +2. ID fields should be `*int64` with `omitempty`. +3. Node ID fields should be `*string` with `omitempty`. +4. Timestamp fields should use `*Timestamp` with `omitempty`. +5. Boolean fields should use `*bool` with `omitempty` to distinguish + between `false` and "not set". + +### URL Tags for Query Parameters + +When defining structs that represent query parameters (sent via URL): + +1. All fields should be non-pointer types with `url` tags: + +```go +type RepositoryContentGetOptions struct { + Ref string `url:"ref,omitempty"` +} + +type ListOptions struct { + Page int `url:"page,omitempty"` + PerPage int `url:"per_page,omitempty"` +} +``` + +2. Use `omitempty` to omit zero values from the query string: + +```go +type RepositoryListOptions struct { + Type string `url:"type,omitempty"` + Sort string `url:"sort,omitempty"` + Direction string `url:"direction,omitempty"` + ListOptions +} +``` + +### Pagination + +The go-github library supports two types of pagination: + +#### Offset-based Pagination + +Use `ListOptions` for APIs that use page/per_page parameters: + +```go +type ListOptions struct { + // For paginated result sets, page of results to retrieve. + Page int `url:"page,omitempty"` + + // For paginated result sets, the number of results to include per page. + PerPage int `url:"per_page,omitempty"` +} +``` + +#### Cursor-based Pagination + +Use `ListCursorOptions` for APIs that use cursor-based pagination: + +```go +type ListCursorOptions struct { + // For paginated result sets, page of results to retrieve. + Page string `url:"page,omitempty"` + + // For paginated result sets, the number of results to include per page. + PerPage int `url:"per_page,omitempty"` + + // For paginated result sets, the number of results per page (max 100), starting from the first matching result. + // This parameter must not be used in combination with last. + First int `url:"first,omitempty"` + + // For paginated result sets, the number of results per page (max 100), starting from the last matching result. + // This parameter must not be used in combination with first. + Last int `url:"last,omitempty"` + + // A cursor, as given in the Link header. If specified, the query only searches for events after this cursor. + After string `url:"after,omitempty"` + + // A cursor, as given in the Link header. If specified, the query only searches for events before this cursor. + Before string `url:"before,omitempty"` + + // A cursor, as given in the Link header. If specified, the query continues the search using this cursor. + Cursor string `url:"cursor,omitempty"` +} +``` + +#### Pagination Best Practices + +1. Embed pagination options in your option structs: + +```go +type RepositoryListOptions struct { + Type string `url:"type,omitempty"` + Sort string `url:"sort,omitempty"` + Direction string `url:"direction,omitempty"` + ListOptions // Embed for pagination +} +``` + +2. The `addOptions` function handles nil options gracefully, so you do not + need to add nil checks for options parameters in your methods. + +3. Use iterators for list methods that support pagination. The library + automatically generates iterator methods (e.g., `ListIter`) for methods + that start with `List` and return a slice. + +4. For APIs with non-standard pagination (e.g., commit files), you may need + to implement custom pagination configuration instead of using the + standard `ListOptions` or `ListCursorOptions`. + +### Common Types + +#### ID Fields + +GitHub API IDs are always `int64`. In request bodies, use non-pointer `int64` +for required fields and `*int64` for optional fields. In response bodies, use +`*int64` with `omitempty`: + +```go +type CreateHostedRunnerRequest struct { + RunnerGroupId int64 `json:"runner_group_id"` // Required, non-pointer +} + +type Repository struct { + ID *int64 `json:"id,omitempty"` // Optional in response, pointer + // ... +} +``` + +#### Node ID Fields + +Node IDs are always `string`: + +```go +type Repository struct { + ID *int64 `json:"id,omitempty"` + NodeID *string `json:"node_id,omitempty"` + // ... +} +``` + +#### Timestamp Fields + +Use the `Timestamp` type for all date/time fields: + +```go +type Repository struct { + CreatedAt *Timestamp `json:"created_at,omitempty"` + UpdatedAt *Timestamp `json:"updated_at,omitempty"` + PushedAt *Timestamp `json:"pushed_at,omitempty"` + // ... +} +``` + +### Generation Patterns + +The go-github repository uses code generation for several purposes: + +#### Generated Accessors + +The `github-accessors.go` file contains generated getter and setter methods +for struct fields. These are generated automatically and should not be edited +manually. + +#### Generated Iterators + +Iterator methods are automatically generated for list methods that: +1. Start with `List` +2. Return a slice +3. Accept pagination options + +For example, `RepositoriesService.List` automatically gets a `ListIter` method. + +#### Generated Documentation + +Documentation links are automatically generated from `openapi_operations.yaml`. +Run `script/generate.sh` to update these links. + ## Metadata GitHub publishes [OpenAPI descriptions of their API][]. We use these @@ -290,21 +662,6 @@ section for more information. **script/test.sh** runs tests on all modules. -## Other notes on code organization - -Currently, everything is defined in the main `github` package, with API methods -broken into separate service objects. These services map directly to how -the [GitHub API documentation][] is organized, so use that as your guide for -where to put new methods. - -Code is organized in files also based pretty closely on the GitHub API -documentation, following the format `{service}_{api}.go`. For example, methods -defined at live in -[repos_hooks.go][]. - -[GitHub API documentation]: https://docs.github.com/en/rest -[repos_hooks.go]: https://github.com/google/go-github/blob/master/github/repos_hooks.go - ## Maintainer's Guide (These notes are mostly only for people merging in pull requests.)