Skip to content

refactor!: Pass the secret scanning body types by value and rename them to ...Request - #4548

Open
JamBalaya56562 wants to merge 3 commits into
google:masterfrom
JamBalaya56562:refactor/3644-secret-scanning-request-types
Open

JamBalaya56562 wants to merge 3 commits into
google:masterfrom
JamBalaya56562:refactor/3644-secret-scanning-request-types

Conversation

@JamBalaya56562

@JamBalaya56562 JamBalaya56562 commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Updates #3644

This converts the two remaining SecretScanningService body types, removing 4 entries from the paramcheck exception lists in .golangci.yml (2 from body-allowed-pointer-types and 2 from body-allowed-wrong-names).

Checking the types against the OpenAPI descriptions turned up two gaps that are fixed in separate commits:

  1. The request schema for PATCH /repos/{owner}/{repo}/secret-scanning/alerts/{alert_number} has five properties (state, resolution, resolution_comment, assignee, validity) with anyOf: [{required: [state]}, {required: [assignee]}, {required: [validity]}]. SecretScanningAlertUpdateOptions only had the first three and declared State string \json:"state"`, so an assignee-only or validity-only update was not possible. Stateis now*stringwithomitempty, and AssigneeandValidityare added. As elsewhere in this package (e.g.IssueRequest.Assignee), omitemptycannot express the schema's "set tonull` to clear" case; that is unchanged here.
  2. The secret scanning alert response schemas have assigned_to, closure_request_comment, closure_request_reviewer, closure_request_reviewer_comment, provider, provider_slug and metadata, none of which were on SecretScanningAlert. They are added as optional fields, with a new SecretScanningAlertMetadata type for the metadata entries.

UpdateSecretScanningPatternConfigsRequest stays shared between UpdatePatternConfigsForEnterprise and UpdatePatternConfigsForOrg; the two request schemas are identical. The pattern configuration tests now also assert the request body, which they did not before.

BREAKING CHANGE: SecretScanningAlertUpdateOptions and SecretScanningPatternConfigsUpdateOptions are renamed to UpdateSecretScanningAlertRequest and UpdateSecretScanningPatternConfigsRequest; SecretScanningService.UpdateAlert, UpdatePatternConfigsForEnterprise and UpdatePatternConfigsForOrg now take them by value instead of by pointer; UpdateSecretScanningAlertRequest.State is now *string.

Adds `assigned_to`, `closure_request_comment`, `closure_request_reviewer`,
`closure_request_reviewer_comment`, `provider`, `provider_slug` and `metadata`
from the secret scanning alert response schemas.
…ions` and make `State` optional

The request schema for PATCH /repos/{owner}/{repo}/secret-scanning/alerts/{alert_number}
requires any one of `state`, `assignee` or `validity`, so `state` is no longer
sent unconditionally.
…em to `...Request`

`SecretScanningAlertUpdateOptions` and `SecretScanningPatternConfigsUpdateOptions`
are renamed to `UpdateSecretScanningAlertRequest` and
`UpdateSecretScanningPatternConfigsRequest`, and the three `SecretScanningService`
methods taking them now take them by value. This removes both types from both
`paramcheck` exception lists in `.golangci.yml`.
@gmlewis gmlewis added NeedsReview PR is awaiting a review before merging. Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). labels Sep 12, 2026
@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.57%. Comparing base (440bc88) to head (e56646c).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4548   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files         197      197           
  Lines       18252    18252           
=======================================
  Hits        17991    17991           
  Misses        261      261           

☔ 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.

Thank you, @JamBalaya56562!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @Not-Dhananjay-Mishra

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). NeedsReview PR is awaiting a review before merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants