feat: Add client API version support

[stevehipwell]

This PR makes the API version used by the client configurable via the new github.WithAPIVersion option function. There is also a new Client.CheckAPIVersion method to validate if the client is currently configured to support a minimum version. I've used this new pattern in the service methods which require the latest API version.

This PR moves the API version selection logic into the client and adds support for setting a min and a max version to constrain the default value. The intended behaviour is that the library maintainers explicitly set the client default version in code and add overrides to the functions where a non-default version is required. Library consumers will then be able to constrain the max and min versions so they get a sentinel error response when they attempt to call an unsupported endpoint.

As it currently stands no behaviour has been changed and this PR just introduces the primitives to extend in future PRs. The first of which I suggest adds github.WithAdvancedServer to set the URLs with correct suffixes and the max version to 2022-11-28.

This is the first part of the implementation for #4237.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.48%. Comparing base (dd6521c) to head (6352685).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4246      +/-   ##
==========================================
+ Coverage   97.46%   97.48%   +0.02%     
==========================================
  Files         190      190              
  Lines       19178    19196      +18     
==========================================
+ Hits        18691    18713      +22     
+ Misses        270      268       -2     
+ Partials      217      215       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[stevehipwell]

@gmlewis @Not-Dhananjay-Mishra I've reworked this whole PR (see updated description), please could you take another look?

[gmlewis]

Is WithAPIVersion going to be added in a follow-up PR?

[stevehipwell]

Is WithAPIVersion going to be added in a follow-up PR?

@gmlewis it could be, but I'm not sure what the purpose of it would be or the actual logic required to implement it "correctly".

While working through these changes I realised that the go-github code couples an implementation to an API version. So by setting an immutable default version that can be overridden on a per endpoint basis I kept the existing logic but made it easier to change how this works in the future. Then by adding min and max versions we can support constrained use cases (e.g. GHAS) without increasing the maintenance burden. It also leaves the door open for more complex patterns in the future (e.g. branching logic), if it's required.

[gmlewis]

Is WithAPIVersion going to be added in a follow-up PR?

@gmlewis it could be, but I'm not sure what the purpose of it would be or the actual logic required to implement it "correctly".

While working through these changes I realised that the go-github code couples an implementation to an API version. So by setting an immutable default version that can be overridden on a per endpoint basis I kept the existing logic but made it easier to change how this works in the future. Then by adding min and max versions we can support constrained use cases (e.g. GHAS) without increasing the maintenance burden. It also leaves the door open for more complex patterns in the future (e.g. branching logic), if it's required.

Yes, my original understanding was that there was no need to concern the user over API versions at all, and we would simply update this client library as the versions changed. I don't even understand why we need a "min" or "max". I thought we would just keep each endpoint in sync with the versions that the GitHub server supplies.

[stevehipwell]

Yes, my original understanding was that there was no need to concern the user over API versions at all, and we would simply update this client library as the versions changed. I don't even understand why we need a "min" or "max". I thought we would just keep each endpoint in sync with the versions that the GitHub server supplies.

The simple first improvement is that you get a sentinel error when the constraints are set and you attempt to use an unsupported endpoint. The second benefit is to be able to design a pattern that allows us to update endpoints for DotCom without being blocked by GHAS; I think we could take a look at the get content functionality to see what this would look like.

As a more abstract concern, I predict that the next new API version is going to be along sooner than 4 years from now.

IMHO the version setting functionality (currently WithVersion) is the thing that likely needs to change to really make use of the new version constraints as the current pattern isn't expressive enough to model the actual system.