From 6fcb261b513138eda84a4a5cd1dded4769539849 Mon Sep 17 00:00:00 2001 From: jlaportebot Date: Fri, 8 May 2026 18:09:38 -0400 Subject: [PATCH] docs: extend CONTRIBUTING.md with code guidelines Add comprehensive section documenting common code patterns and conventions: - Naming conventions (files, receivers, types, methods, variables) - JSON tags for request bodies (required vs optional fields, omitzero) - URL tags for query parameters (non-pointer fields with omitempty) - Pagination patterns (ListOptions, ListCursorOptions, best practices) - Common types (ID, NodeID, Timestamp, boolean fields) - Generation patterns (accessors, iterators, documentation, linter rules) This addresses issue #4023 and helps new contributors understand the codebase better, reducing review burden and improving code consistency. --- CONTRIBUTING.md | 349 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 349 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1c3d86f7a4d..0538baaa6fc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -199,6 +199,355 @@ description. [Go Doc Comments]: https://go.dev/doc/comment +## 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. + +### 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`. + +#### 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) are named after the method +they modify, with the suffix `Options`: + +```go +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"` +} +``` + +#### 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"` + // ... +} +``` + +#### 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 +- `Edit` - Edit an existing resource (alternative to Update) + +Common local variable names: +- `ctx` - Context +- `u` - URL string +- `req` - HTTP request +- `resp` - HTTP response +- `result` - Result from API call +- `err` - Error + +#### 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 + +#### Enterprise and Organization Scoped Methods + +Methods that operate on enterprise or organization resources include the scope +in their name: + +```go +// Enterprise-scoped methods +func (s *EnterpriseService) GetLicenseInfo(ctx context.Context) (*LicenseInfo, *Response, error) + +// Organization-scoped methods +func (s *OrganizationsService) ListMembers(ctx context.Context, org string, opts *OrganizationListMembersOptions) ([]*User, *Response, error) +``` + +#### 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 for 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 slices** where you want to omit empty values + (not just nil): + +```go +type ActivityNotificationOptions struct { + SubjectType string `json:"subject_type,omitempty"` + Subject *string `json:"subject,omitempty"` + LastReadAt Timestamp `json:"last_read_at,omitzero"` +} +``` + +### 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. **Handle nil options** in your methods: + +```go +func (s *RepositoriesService) List(ctx context.Context, user string, opts *RepositoryListOptions) ([]*Repository, *Response, error) { + if opts == nil { + opts = &RepositoryListOptions{} + } + // ... +} +``` + +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. + +### Common Types + +These types are used consistently throughout the codebase: + +#### ID Fields + +GitHub API IDs are always `int64`: + +```go +type Repository struct { + ID *int64 `json:"id,omitempty"` + // ... +} + +type User struct { + ID *int64 `json:"id,omitempty"` + // ... +} +``` + +#### 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"` + // ... +} +``` + +#### Boolean Fields + +Boolean fields are always `*bool` (pointer to bool) to distinguish between +`false` and `not set`: + +```go +type Repository struct { + Private *bool `json:"private,omitempty"` + Fork *bool `json:"fork,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. + +#### Linter Rules + +The repository uses custom linter rules to enforce consistency: +- `sliceofpointers` - Ensures slice fields use pointers +- `structfield` - Ensures struct fields follow naming conventions + ## Metadata GitHub publishes [OpenAPI descriptions of their API][]. We use these