Skip to content

docs: Extend CONTRIBUTING.md with code guidelines#4339

Merged
gmlewis merged 2 commits into
google:masterfrom
jlaportebot:docs/extend-contributing-guidelines
Jun 29, 2026
Merged

docs: Extend CONTRIBUTING.md with code guidelines#4339
gmlewis merged 2 commits into
google:masterfrom
jlaportebot:docs/extend-contributing-guidelines

Conversation

@jlaportebot

Copy link
Copy Markdown
Contributor

Extends CONTRIBUTING.md with comprehensive code guidelines for contributors, including:

  • Code style and formatting standards
  • Testing requirements and patterns
  • Documentation expectations
  • Commit message conventions
  • Code review guidelines

Addresses feedback from maintainers on the initial submission.

- Add Code Guidelines section with naming conventions, type conventions, JSON tags,
  URL tags, pagination, and common types
- Move Code Comments under Code Guidelines
- Remove duplicate content from Submitting a patch, Metadata, Scripts sections
- Fix gofmt formatting for all code snippets
- Incorporate reviewer feedback from google#4202 (alexandear, stevehipwell)
- Address gmlewis feedback on keeping CONTRIBUTING.md concise
@jlaportebot

Copy link
Copy Markdown
Contributor Author

CLA Fix Summary

Closed PR #4299 (and #4202) due to CLA check failures caused by:

  1. 2867 commits in the PR history — git filter-branch accidentally rewrote the entire repo history instead of just the PR branch
  2. Mixed email addresses — Author was jlaportebot@users.noreply.github.com but Committer was jlaportebot@gmail.com (CLA only signed for the noreply email)
  3. Invalid Co-authored-by trailer in original commit: Co-authored-by: review feedback from google/go-github maintainers (missing email in angle brackets)

What I did to fix:

  • Reset branch to upstream/master (clean base)
  • Cherry-picked the CONTRIBUTING.md changes as a single commit
  • Set both Author and Committer to jlaportebot@users.noreply.github.com (the email with signed Google CLA)
  • Removed the problematic Co-authored-by trailer

Result: PR #4339 now has 1 commit with clean CLA verification ✅

The CONTRIBUTING.md content is identical to what was in #4299 — all reviewer feedback incorporated (alexandear, stevehipwell, gmlewis). Ready for review!

@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Jun 28, 2026
@codecov

codecov Bot commented Jun 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.50%. Comparing base (4b55679) to head (ed1e6bf).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4339   +/-   ##
=======================================
  Coverage   97.50%   97.50%           
=======================================
  Files         193      193           
  Lines       19451    19451           
=======================================
  Hits        18965    18965           
  Misses        269      269           
  Partials      217      217           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gmlewis gmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, otherwise LGTM.

Comment thread CONTRIBUTING.md Outdated
Comment on lines +174 to +185
...
}
```
And the returned type `Repository` will have comments like this:

```go
// Repository represents a GitHub repository.
type Repository struct {
ID *int64 `json:"id,omitempty"`
ID *int64 `json:"id,omitempty"`
NodeID *string `json:"node_id,omitempty"`
Owner *User `json:"owner,omitempty"`
// ...
Owner *User `json:"owner,omitempty"`
...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, these changes to the code sections are a regression and the new formatting looks worse than the original. I think that the changes to the new lines 174-185 should be reverted.

Revert the formatting changes to the code examples in the Code Comments
section to match the original style with tabs and // ... comments,
as requested in review feedback from gmlewis.
@jlaportebot

Copy link
Copy Markdown
Contributor Author

I've addressed the nit from the review - reverted the code formatting changes in the Code Comments examples to match the original style with tabs and // ... comments. The review was on the previous commit (130d1aa) and this is now on the new commit (ed1e6bf). The CLA check passes and all checks are green. Ready for re-review!

@gmlewis gmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stevehipwell stevehipwell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I do still think that using the Repository struct in the example for a response object is unclear as it's two most significant fields (ID & Name) are pointers when they are required and by the coding standards logic should not be.

@gmlewis

gmlewis commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

I do still think that using the Repository struct in the example for a response object is unclear as it's two most significant fields (ID & Name) are pointers when they are required and by the coding standards logic should not be.

This is an orthogonal issue. I believe it was originally done this way because, depending upon the endpoint where either *Repository or []*Repository was being returned, they were emperically found to not be populated by the GitHub server, and therefore were made pointers so that the client could more easily figure out if the JSON response actually included these fields.

Feel free to file a separate issue so that someone can investigate if this is no longer the case in any of the DotCom or current or legacy Enterprise versions of GitHub so that they can be changed.

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Jun 29, 2026
@gmlewis gmlewis merged commit 4498e09 into google:master Jun 29, 2026
15 checks passed
@stevehipwell

Copy link
Copy Markdown
Contributor

This is an orthogonal issue. I believe it was originally done this way because, depending upon the endpoint where either *Repository or []*Repository was being returned, they were emperically found to not be populated by the GitHub server, and therefore were made pointers so that the client could more easily figure out if the JSON response actually included these fields.

Feel free to file a separate issue so that someone can investigate if this is no longer the case in any of the DotCom or current or legacy Enterprise versions of GitHub so that they can be changed.

@gmlewis I was very careful not to say that the Repository implementation wasn't correct, just that it made the example confusing compared to uisng a type where the identity fields weren't pointers.

eleboucher pushed a commit to eleboucher/forgesync that referenced this pull request Jul 7, 2026
… v89.0.0) (#16)

This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [github.com/google/go-github/v88](https://github.com/google/go-github) | `v88.0.0` → `v89.0.0` | ![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgoogle%2fgo-github%2fv88/v89.0.0?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgoogle%2fgo-github%2fv88/v88.0.0/v89.0.0?slim=true) |

---

### Release Notes

<details>
<summary>google/go-github (github.com/google/go-github/v88)</summary>

### [`v89.0.0`](https://github.com/google/go-github/releases/tag/v89.0.0)

[Compare Source](google/go-github@v88.0.0...v89.0.0)

This release contains the following breaking API changes:

- refactor!: Pass `DeploymentRequest` and `DeploymentStatusRequest` by value ([#&#8203;4361](google/go-github#4361))
  BREAKING CHANGE: `CreateDeployment` and `CreateDeploymentStatus` now take `DeploymentRequest` and `DeploymentStatusRequest` by value; `DeploymentRequest.Ref` and `DeploymentStatusRequest.State` are now `string`, and `DeploymentRequest.RequiredContexts` is now `[]string`.
- refactor!: Pass `HookConfig` by value and rename `EditHookConfiguration` to `UpdateHookConfiguration` ([#&#8203;4360](google/go-github#4360))
  BREAKING CHANGE: `EditHookConfiguration` is renamed to `UpdateHookConfiguration` on `RepositoriesService` and `OrganizationsService`; these methods and `AppsService.UpdateHookConfig` now take `HookConfig` by value.
- refactor!: Pass `OIDCSubjectClaimCustomTemplate` by value in the OIDC subject-claim Set methods ([#&#8203;4340](google/go-github#4340))
  BREAKING CHANGE: `SetOrgOIDCSubjectClaimCustomTemplate` and `SetRepoOIDCSubjectClaimCustomTemplate` now take their `body` params by value.
- feat!: Refactor actions variables to pass request by value ([#&#8203;4346](google/go-github#4346))
  BREAKING CHANGE: `ActionsService` methods involving variables have new params and return values.
- feat!: Replace actions env secret endpoints ([#&#8203;4335](google/go-github#4335))
  BREAKING CHANGE: `ActionsService` methods involving secrets have new params and return values.
- refactor!: Pass `CreateJITConfigRequest` by value and rename `Generate*JITConfig` to `Create*JITConfig` ([#&#8203;4337](google/go-github#4337))
  BREAKING CHANGE: the JIT config methods are renamed from `Generate*JITConfig` to `Create*JITConfig`, and they now take `CreateJITConfigRequest` (renamed from `GenerateJITConfigRequest`) by value instead of by pointer.
- refactor!: Pass release-notes and asset params by value and rename `EditReleaseAsset` to `UpdateReleaseAsset` ([#&#8203;4336](google/go-github#4336))
  BREAKING CHANGE: `GenerateReleaseNotes` now takes `GenerateNotesRequest` by value (renamed from `GenerateNotesOptions`); `EditReleaseAsset` is renamed to `UpdateReleaseAsset` and takes `UpdateReleaseAssetRequest` by value.
- refactor!: Pass release params by value and rename `EditRelease` to `UpdateRelease` ([#&#8203;4329](google/go-github#4329))
  BREAKING CHANGE: `CreateRelease` & `UpdateRelease` now take `RepositoryRelease` by value; `EditRelease` is renamed to `UpdateRelease`.
- refactor!: Pass `GistsService` required params by value ([#&#8203;4320](google/go-github#4320))
  BREAKING CHANGE: `GistsService` methods now pass required params by-value instead of by-ref.
- fix!: Send request body in SCIM update methods ([#&#8203;4315](google/go-github#4315))
  BREAKING CHANGE: `UpdateProvisionedOrgMembership` and `UpdateAttributeForSCIMUser` params and return values changed.
- fix!: Fix `LicenseStatus` response and `Supportkey` type ([#&#8203;4297](google/go-github#4297))
  BREAKING CHANGE: `LicenseStatus.SupportKey` type changed from `*string` to `*bool` and `License` return type is no longer a slice.
- fix!: Enterprise App installation repos options structs ([#&#8203;4298](google/go-github#4298))
  BREAKING CHANGE: `SelectedRepositoryIDs []int64` is now `Repositories []string` in `*AppInstallationRepositoriesOptions`.

...and the following additional changes:

- chore: Bump version of go-github to v89.0.0 ([#&#8203;4369](google/go-github#4369))
- feat: Add user team membership fields ([#&#8203;4347](google/go-github#4347))
- chore: Add deprecated flag for unused metadata ([#&#8203;4367](google/go-github#4367))
- docs: Add AGENTS.md and review-feedback tip ([#&#8203;4368](google/go-github#4368))
- chore: Mark removed billing endpoints as deprecated ([#&#8203;4362](google/go-github#4362))
- chore: Validate metadata ([#&#8203;4358](google/go-github#4358))
- chore: Bump golang.org/x/tools to v0.47.0 ([#&#8203;4357](google/go-github#4357))
- build(deps): Bump actions/setup-go from 6.4.0 to 6.5.0 in the actions group ([#&#8203;4349](google/go-github#4349))
- build(deps): Bump the go\_modules group in /example ([#&#8203;4345](google/go-github#4345))
- feat: Add `AccessSource` to `Team` ([#&#8203;4344](google/go-github#4344))
- docs: Update `CONTRIBUTING.md` ([#&#8203;4341](google/go-github#4341))
- build(deps): Bump golangci/golangci-lint-action from 9.2.1 to 9.3.0 ([#&#8203;4342](google/go-github#4342))
- docs: Extend CONTRIBUTING.md with code guidelines ([#&#8203;4339](google/go-github#4339))
- fix: AuditEntry fields `org` and `org_id` can be an array ([#&#8203;4333](google/go-github#4333))
- build(deps): Bump actions/checkout from 6.0.3 to 7.0.0 in the actions group ([#&#8203;4332](google/go-github#4332))
- feat: Add code quality findings support ([#&#8203;4330](google/go-github#4330))
- chore: Update `openapi_operations.yaml` ([#&#8203;4331](google/go-github#4331))
- chore: Replace `time.Date` with `referenceTime` ([#&#8203;4325](google/go-github#4325))
- test: Refactor tests for `ProjectV2Item.UnmarshalJSON` ([#&#8203;4323](google/go-github#4323))
- chore: Remove obsolete test/fields utility ([#&#8203;4322](google/go-github#4322))
- feat: Add Issue Dependencies API support ([#&#8203;4130](google/go-github#4130))
- fix: Set `GetBody` on uploads for HTTP/2 retry ([#&#8203;4318](google/go-github#4318))
- chore: Remove deleted account from REVIEWERS ([#&#8203;4317](google/go-github#4317))
- build(deps): Bump golang.org/x/crypto from 0.52.0 to 0.53.0 in /example ([#&#8203;4305](google/go-github#4305))
- build(deps): Bump codecov/codecov-action from 6.0.1 to 7.0.0 ([#&#8203;4303](google/go-github#4303))
- build(deps): Bump golang.org/x/net from 0.55.0 to 0.56.0 in /scrape ([#&#8203;4302](google/go-github#4302))
- build(deps): Bump golang.org/x/term from 0.43.0 to 0.44.0 in /example ([#&#8203;4304](google/go-github#4304))
- build(deps): Bump golang.org/x/tools from 0.45.0 to 0.46.0 in /tools ([#&#8203;4314](google/go-github#4314))
- build(deps): Bump github.com/bradleyfalzon/ghinstallation/v2 from 2.18.0 to 2.19.0 in /example ([#&#8203;4306](google/go-github#4306))
- refactor: Fix parameters in method endpoints ([#&#8203;4300](google/go-github#4300))
- feat: Add enterprise billing usage endpoints and response types ([#&#8203;4288](google/go-github#4288))
- build(deps): Bump golang.org/x/sync from 0.20.0 to 0.21.0 in /tools ([#&#8203;4294](google/go-github#4294))
- build(deps): Bump github.com/getkin/kin-openapi from 0.139.0 to 0.140.0 in /tools ([#&#8203;4295](google/go-github#4295))
- build(deps): Bump actions/checkout from 6.0.2 to 6.0.3 in the actions group ([#&#8203;4293](google/go-github#4293))
- fix: Reject URL path segments containing percent-encoded dots ([#&#8203;4291](google/go-github#4291))
- fix: Comparison of durations in `AbuseRateLimitError.Is` ([#&#8203;4292](google/go-github#4292))
- test: Replace `&x` variables with inline `Ptr(value)` calls ([#&#8203;4289](google/go-github#4289))
- feat: Inject OpenAPI deprecations safely ([#&#8203;4286](google/go-github#4286))
- chore: Remove 'munlicode' from REVIEWERS ([#&#8203;4287](google/go-github#4287))
- feat: Add `GetOrgAICreditUsage` and  `GetUserAICreditUsage` endpoints ([#&#8203;4282](google/go-github#4282))
- test: Fix test names and error messages ([#&#8203;4284](google/go-github#4284))
- test: Use `testJSONBody` helper for request body assertions ([#&#8203;4283](google/go-github#4283))
- feat: Add pull request fields to `Repository` ([#&#8203;4268](google/go-github#4268))
- build(deps): Bump golangci/golangci-lint-action from 9.2.0 to 9.2.1 ([#&#8203;4272](google/go-github#4272))
- build(deps): Bump go.opentelemetry.io/otel to v1.44.0 ([#&#8203;4280](google/go-github#4280))
- build(deps): Bump golang.org/x/net from 0.54.0 to 0.55.0 in /scrape ([#&#8203;4271](google/go-github#4271))
- build(deps): Bump golang.org/x/crypto from 0.51.0 to 0.52.0 in /example ([#&#8203;4273](google/go-github#4273))
- fix: Handle missing reviewer type in `RequiredReviewer` unmarshal ([#&#8203;4270](google/go-github#4270))
- test: Remove redundant Marshal tests ([#&#8203;4266](google/go-github#4266))
- chore: Update `openapi_operations.yaml` ([#&#8203;4265](google/go-github#4265))
- feat: Add support for GitHub Code Quality API ([#&#8203;4260](google/go-github#4260))
- build(deps): Bump github.com/getkin/kin-openapi from 0.138.0 to 0.139.0 in /tools ([#&#8203;4261](google/go-github#4261))
- chore: Fix zizmor security issues in GHA workflows ([#&#8203;4259](google/go-github#4259))
- feat: Add support for getting Copilot cloud agent configuration ([#&#8203;4241](google/go-github#4241))
- feat: Add client API version support ([#&#8203;4246](google/go-github#4246))
- test: Add secret scanning marshal tests ([#&#8203;4252](google/go-github#4252))
- lint: Improve `extraneousnew` linter to catch unnecessary use of value var ([#&#8203;4249](google/go-github#4249))
- build(deps): Bump codecov/codecov-action from 6.0.0 to 6.0.1 ([#&#8203;4248](google/go-github#4248))
- chore: Bump go-github from v87 to v88 in /scrape ([#&#8203;4247](google/go-github#4247))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL21ham9yIl19-->

Reviewed-on: https://git.erwanleboucher.dev/eleboucher/forgesync/pulls/16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants