-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Support Github Advanced Security repository settings #2133
Conversation
@gmlewis let me know if you'd like me to add any other tests other than the generated ones. |
aeb8c43
to
9b18240
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @asvoboda .
In addition to the minor tweaks I've requested, it would be nice to add the new fields to any existing tests just to test the marshaling and unmarshaling.
Please try not to use force-push in this repo's PRs because it makes it more challenging for reviewers to see what changed since their last code review. |
Codecov Report
@@ Coverage Diff @@
## master #2133 +/- ##
=======================================
Coverage 97.78% 97.78%
=======================================
Files 112 112
Lines 9970 9976 +6
=======================================
+ Hits 9749 9755 +6
Misses 154 154
Partials 67 67
Continue to review full report at Codecov.
|
@@ -360,7 +360,7 @@ func TestRepositoriesService_Get(t *testing.T) { | |||
mux.HandleFunc("/repos/o/r", func(w http.ResponseWriter, r *http.Request) { | |||
testMethod(t, r, "GET") | |||
testHeader(t, r, "Accept", strings.Join(wantAcceptHeaders, ", ")) | |||
fmt.Fprint(w, `{"id":1,"name":"n","description":"d","owner":{"login":"l"},"license":{"key":"mit"}}`) | |||
fmt.Fprint(w, `{"id":1,"name":"n","description":"d","owner":{"login":"l"},"license":{"key":"mit"},"security_and_analysis":{"advanced_security":{"status":"enabled"},"secret_scanning":{"status":"enabled"}}}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know if there is a better place for this, or if you'd like a separate test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @asvoboda !
LGTM.
Merging.
Adds new fields from Advanced Security, as documented in https://docs.github.com/en/rest/reference/repos#update-a-repository and https://docs.github.com/en/get-started/learning-about-github/about-github-advanced-security
Fixes #2132