Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Sep 19, 2023

Conversation

gmlewis
Copy link
Collaborator

@gmlewis gmlewis commented Sep 19, 2023

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>
@gmlewis gmlewis added the NeedsReview PR is awaiting a review before merging. label Sep 19, 2023
@codecov
Copy link

codecov bot commented Sep 19, 2023

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 Changed Coverage Δ
github/secret_scanning.go 100.00% <ø> (ø)

Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@gmlewis gmlewis 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
Collaborator Author

gmlewis commented Sep 19, 2023

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

@jentfoo
Copy link

jentfoo commented Sep 19, 2023

Thank you for the help in fixing this @gmlewis!

@gmlewis
Copy link
Collaborator Author

gmlewis commented Sep 19, 2023

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
Collaborator Author

gmlewis commented Sep 19, 2023

Thank you, @jentfoo !
Merging.

@gmlewis gmlewis removed the NeedsReview PR is awaiting a review before merging. label Sep 19, 2023
@gmlewis gmlewis merged commit b45ef89 into google:master Sep 19, 2023
10 checks passed
@gmlewis gmlewis deleted the bugfix/i2871-secret-scanning-alerts branch September 19, 2023 14:32
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).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants