fix!: Fix LicenseStatus response and Supportkey type#4297
Conversation
Supportkey type
Supportkey typeLicenseStatus response and Supportkey type
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4297 +/- ##
=======================================
Coverage 97.46% 97.47%
=======================================
Files 192 192
Lines 19258 19310 +52
=======================================
+ Hits 18770 18822 +52
Misses 270 270
Partials 218 218 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @s4mur4i!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear - @zyfy29 - @Not-Dhananjay-Mishra
| // | ||
| //meta:operation GET /manage/v1/config/license | ||
| func (s *EnterpriseService) License(ctx context.Context) ([]*LicenseStatus, *Response, error) { | ||
| func (s *EnterpriseService) License(ctx context.Context) (*LicenseStatus, *Response, error) { |
There was a problem hiding this comment.
Let's add a comment to the function that GitHub documentation is wrong about the response.
There was a problem hiding this comment.
Their last response was:
We’ll treat this as a documentation/schema mismatch and route it for review so the example can be corrected to match the actual GHES behavior.
Eventually it should be corrected, in that case wouldn't that cause confusion?
| // License gets the current license information for the GitHub Enterprise instance. | ||
| // | ||
| // GitHub API docs: https://docs.github.com/enterprise-server@3.21/rest/enterprise-admin/manage-ghes#get-the-enterprise-license-information | ||
| // | ||
| //meta:operation GET /manage/v1/config/license | ||
| func (s *EnterpriseService) License(ctx context.Context) ([]*LicenseStatus, *Response, error) { | ||
| // Current Docs shouw incorrect return type. They list as [{...}] but actual is {...} | ||
| // TODO: Remove comment after github updates schema documentation |
There was a problem hiding this comment.
I propose adding a note:
// License gets the current license information for the GitHub Enterprise instance.
//
// NOTE: The GitHub documentation incorrectly shows the return type as a list ([{...}]),
// but the actual response is a single object ({...}).
// TODO: Remove this note once GitHub corrects the schema documentation.
//
// GitHub API docs: https://docs.github.com/enterprise-server@3.21/rest/enterprise-admin/manage-ghes#get-the-enterprise-license-information
//
//meta:operation GET /manage/v1/config/licenseThere was a problem hiding this comment.
I cannot accept this, should I just update with commit?
Co-authored-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
BREAKING CHANGE:
LicenseStatus.SupportKeytype changed from*stringto*boolandLicensereturn type is no longer a slice.Closes: #4296
License status response was implemented according to documentation, which is not according to actual response.
Fix:
Corrected the response
Fixed incorrect data type for SupportKey
Fixed tests