Skip to content

Fix SecretScanning API by switching arguments from url to json - #2934

Merged
gmlewis merged 2 commits into
google:masterfrom
gmlewis:bugfix/i2871-secret-scanning-alerts
Sep 19, 2023
Merged

Fix SecretScanning API by switching arguments from url to json#2934
gmlewis merged 2 commits into
google:masterfrom
gmlewis:bugfix/i2871-secret-scanning-alerts

Conversation

@gmlewis

Copy link
Copy Markdown
Collaborator

Closes: #2871.

This PR fixes the endpoint:
https://docs.github.com/en/enterprise-server@3.5/rest/secret-scanning/secret-scanning#update-a-secret-scanning-alert
by sending its parameters in the body instead of as URL query parameters.

I was unable to edit #2871 directly, so this is a replacement.
Thank you, @jentfoo for the bug report and initial PR!

Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@gmlewisgmlewis added the NeedsReview PR is awaiting a review before merging. label Sep 19, 2023
@codecov

codecovBot commented Sep 19, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2934 (9a30ff0) into master (0e8dfae) will not change coverage.
Report is 1 commits behind head on master.
The diff coverage is n/a.

@@ Coverage Diff @@## master #2934 +/- ##
=======================================
Coverage 98.17% 98.17% =======================================
Files 143 143 Lines 12597 12597 =======================================
Hits 12367 12367 Misses 156 156 Partials 74 74 
Files ChangedCoverage Δ
github/secret_scanning.go100.00% <ø> (ø)

Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@gmlewisgmlewis added the Breaking API Change PR will require a bump to the major version num in next release. Look here to see the change(s). label Sep 19, 2023
@gmlewis

Copy link
Copy Markdown
CollaboratorAuthor

Awaiting LGTM+Approval from any other contributor to this repo before merging.

@jentfoo

Copy link
Copy Markdown

Thank you for the help in fixing this @gmlewis!

@gmlewis

Copy link
Copy Markdown
CollaboratorAuthor

Thank you for the help in fixing this @gmlewis!

My pleasure. Thank you, @jentfoo for the bug report and your help tracking down the problem!
Would you mind giving an LGTM+Approval so I can merge this?

@gmlewis

Copy link
Copy Markdown
CollaboratorAuthor

Thank you, @jentfoo !
Merging.

@gmlewisgmlewis removed the NeedsReview PR is awaiting a review before merging. label Sep 19, 2023
@gmlewis
gmlewis merged commit b45ef89 into google:masterSep 19, 2023
@gmlewis
gmlewis deleted the bugfix/i2871-secret-scanning-alerts branch September 19, 2023 14:32
jlaportebot added a commit to jlaportebot/go-github that referenced this pull request Jun 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking API ChangePR will require a bump to the major version num in next release. Look here to see the change(s).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@gmlewis@jentfoo