Support new GitHub v3 API calendar-based versioning #2581
Conversation
Signed-off-by: Glenn Lewis <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2581 +/- ##
=======================================
Coverage 97.98% 97.98%
=======================================
Files 125 125
Lines 10875 10894 +19
=======================================
+ Hits 10656 10675 +19
Misses 150 150
Partials 69 69
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Glenn Lewis <[email protected]>
There was a problem hiding this comment.
I think this definitely makes sense for what the go API should look like. In practice, I think we would want per-method version overrides (at least in the core library) to be rare and temporary.
My read of the GitHub docs is that they will be revving the entire API to each new date-based version, even if only a few methods have breaking changes. Other methods will accept the new version with their existing functionality. So when a new date-based version of the GitHub API is released, I think we'll want to:
- update each method that had breaking changes, overriding their per-method API version header. This may happen in one or multiple commits and PRs, and is all done in the main branch.
- once all of the methods with breaking changes have been updated, have a final commit that bumps the default API version, and remove all of the per-method overrides. That would now get a major version bump when the next go-github release is made.
We'll probably want to clearly identify in the README which version of the GitHub API this version of go-github works with (in case users need a specific version). And depending on how frequently GitHub revs their date-based versions, we might eventually have a table of the last go-github version that supported each GitHub API version?
Exporting the NewRequest and Do methods has meant that go-github users could always customize their API calls instead of using the providing method wrappers. Adding RequestOption like you have here keeps that simple and easily accessible, so I really like this.
github/github.go
Outdated
| // RequestOption represents an option that can modify an http.Request. | ||
| type RequestOption func(req *http.Request) | ||
|
|
||
| // WithVersion modifies the GitHub v3 API version for this individual request. |
There was a problem hiding this comment.
maybe change "modifies" to "overrides" or "replaces", just to emphasize that this completely replaces the previous version?
I could imagine other RequestOptions that add a new header without replacing existing ones (for example, if we were to have done something like this for adding the preview Accept headers)
Signed-off-by: Glenn Lewis <[email protected]>
|
Excellent feedback, @willnorris - thank you very much! I've attempted to capture your ideas into the README.md file. |
|
Thank you, @raynigon ! |
Fixes: #2580.
@willnorris - please see if you agree with this approach for supporting the new GitHub v3 API versioning mechanism.
My thought is that when they add breaking API changes, that we will continue to support them, but will now add the
WithVersionoption to each request that needs it.