Remove REST API preview headers that have been promoted out of preview

[andygrunwald]

What type of PR is this?

cleanup

What this PR does / why we need it:

On October 14, 2021, GitHub has promoted 23 preview headers. See October 14, 2021: REST API preview promotions @ GitHub Changelog blog.

This PR will remove 25 (!) preview headers from the codebase, but we keep the two headers that are still in preview by GitHub (mentioned in the blog post):

• application/vnd.github.corsair-preview+json
• application/vnd.github.scarlet-witch-preview+json

Removed headers:

• application/vnd.github.v3.star+json
• application/vnd.github.wyandotte-preview+json
• application/vnd.github.ant-man-preview+json
• application/vnd.github.flash-preview+json
• application/vnd.github.squirrel-girl-preview
• application/vnd.github.mockingbird-preview+json
• application/vnd.github.inertia-preview+json
• application/vnd.github.cloak-preview+json
• application/vnd.github.giant-sentry-fist-preview+json
• application/vnd.github.mercy-preview+json
• application/vnd.github.luke-cage-preview+json
• application/vnd.github.eye-scream-preview
• application/vnd.github.zzzax-preview+json
• application/vnd.github.starfox-preview+json
• application/vnd.github.sombra-preview+json
• application/vnd.github.switcheroo-preview+json
• application/vnd.github.dorian-preview+json
• application/vnd.github.london-preview+json
• application/vnd.github.lydian-preview+json
• application/vnd.github.groot-preview+json
• application/vnd.github.surtur-preview+json
• application/vnd.github.baptiste-preview+json
• application/vnd.github.comfort-fade-preview+json
• application/vnd.github.doctor-strange-preview+json
• application/vnd.github.nebula-preview+json

Why there is a difference of 2 (this PR removes 25 previews, the blog post is mentioning 23)?

Sadly there is no complete list provided in the blogpost of previews that have been removed.
My assumption is that this codebase contained preview headers that have been graduated before.

Special notes for your reviewer:

Size of the Pull Request

This Pull Request is very big and (IMO) hard to review.
Every commit is atomic and removes only one preview.
If you wish to receive 25 Pull Requests (one PR for each preview header), please let me know.

Other obsolete code

By removing the preview headers, I found the following code to be also obsolete:

isComfortFadePreview (incl. tests)

go-github/github/pulls_reviews.go

Line 67 in f664094

func (r *PullRequestReviewRequest) isComfortFadePreview() (bool, error) {

DraftReviewComment fields

go-github/github/pulls_reviews.go

Lines 43 to 47 in f664094

	// The new comfort-fade-preview fields
	StartSide *string `json:"start_side,omitempty"`
	Side *string `json:"side,omitempty"`
	StartLine *int `json:"start_line,omitempty"`
	Line *int `json:"line,omitempty"`

For now, I kept this code in the codebase to not blow it up even more.
Please let me know what you think about it.

Additional documentation, usage docs, etc.:

• October 14, 2021: REST API preview promotions @ GitHub Changelog blog
• GitHub Developer Changes

[codecov]

Codecov Report

Merging #2125 (206432a) into master (d1858a3) will decrease coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2125      +/-   ##
==========================================
- Coverage   97.78%   97.71%   -0.08%     
==========================================
  Files         112      112              
  Lines        9970     9657     -313     
==========================================
- Hits         9749     9436     -313     
  Misses        154      154              
  Partials       67       67              

Impacted Files	Coverage Δ
github/activity_star.go	100.00% <ø> (ø)
github/apps_installation.go	89.65% <ø> (-2.02%)	⬇️
github/authorizations.go	100.00% <ø> (ø)
github/github.go	97.58% <ø> (ø)
github/interactions_orgs.go	100.00% <ø> (ø)
github/interactions_repos.go	100.00% <ø> (ø)
github/issues.go	96.87% <ø> (-0.19%)	⬇️
github/issues_comments.go	100.00% <ø> (ø)
github/issues_events.go	100.00% <ø> (ø)
github/issues_timeline.go	100.00% <ø> (ø)
... and 24 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1858a3...206432a. Read the comment docs.

[andygrunwald]

codecov seems to fail due to lower test coverage.
I am waiting for maintainers' feedback before taking a deeper look into this.

[gmlewis]

Thank you, @andygrunwald .

Lately (within the last year or so), we've received a number of complaints from GitHub Enterprise users of this repo that removing custom preview headers are causing their systems to fail because their version of GHE is fixed at a certain version that still requires them to be used.

I don't have all the issue numbers at the moment where this occurred but could probably dig them up when I have more time.

In the meantime, do you know if the GitHub team has any recommendations for us where we support both current top-of-truck public GitHub APIs in addition to supporting (various) prior versions of GitHub Enterprise?

[andygrunwald]

Thanks, @gmlewis.
I clearly miss this context. And it makes totally sense.

I think this seems to be a more strategic decision for this library. Mainly around the question of "Which versions of GHE are supported by this library?".

In the meantime, do you know if the GitHub team has any recommendations for us where we support both current top-of-truck public GitHub APIs in addition to supporting (various) prior versions of GitHub Enterprise?

I don't know yet, but I can check if I get an answer from someone inside GitHub.
Let me come back to you.

[andygrunwald]

@gmlewis I have reached out to GitHub Enterprise Support and mentioned the topic of this Pull Request in particular. Here is what we came up with:

• GitHub confirmed that this is a tricky one
• GitHub doesn't have an official recommendation on how to deal with this
• It's not uncommon for GitHub Enterprise Server customers to run older versions which would probably rely on the preview headers for some time
• While this header promotion is going out for GitHub.com, it may be some time before it even arrives in GitHub Enterprise Server
• There is a list of GitHub Enterprise Server versions officially supported from GitHub at https://docs.github.com/en/enterprise-server@3.2/admin/all-releases
• The release notes for the GitHub Enterprise Server can be found at https://enterprise.github.com/releases

Based on this information, I see a few possible options

1. We keep all preview headers "indefinitely"

go-github implements all (ever) existing preview headers, even if they have been graduated, into the respective API calls. Those headers would be kept "indefinitely". "indefinitely" is up to definition if, in x years, cleanup should be done to reduce complexity.

As far as I know, the implementation on GitHub (Server) side is good enough to not trigger side effects/unexpected behavior when (additional/not respected) headers are sent.

Upsides:

• A lot of GitHub Enterprise Servers are supported (even very old ones)
• Constant maintenance is not required (sure, new features need to be added)

Downsides:

• Bundling code that is (depending on the context) [maybe] not needed

2. Supporting the latest state of GitHub.com + official GitHub Enterprise Server versions

go-github follows the Changelogs at https://enterprise.github.com/releases (for new Enterprise versions), https://docs.github.com/en/enterprise-server@3.2/admin/all-releases (for supported Enterprise versions) and https://github.blog/changelog/ (for changes to Github.com) and updates the library according to the headers/features that match all three.

Upsides:

• The library only sends the headers that are neeeded

Downsides:

• Needs constant maintenance to stay up to date

3. Implementing a "Version Check" for GitHub Enterprise Servers

go-github could check what version we have on the server-side (cloud, enterprise v3.0, ...) and set the responding preview headers accordingly.

Downsides:

• Complexity in implementation
• Needs constant maintenance to stay up to date

My 2 cents

Solution 1. We keep all preview headers "indefinitely" seems to be the one with the least effort, but also the least downsides.
In this case, I would suggest adding a bigger comment block above the preview header listing, explaining the situation and providing the respective context on why we keep all preview headers.

@gmlewis Do you see additional options? What do you think about this?

[gmlewis]

Excellent job, @andygrunwald ! Thank you so much for contacting GitHub tech support and for summarizing the situation perfectly.

I'm in agreement with you, and if you wouldn't mind, I think a PR from you explaining the situation clearly as you recommend would be the very best outcome.

[andygrunwald]

Sounds good @gmlewis.
Let me take this from here and I will create a PR in the next days.

If you don't mind, I would like to keep this PR open until the new PR is there and replaces this one.

One question:
Should further preview header removals also be reverted? Or are we good with the current set of API Preview Headers? (e.g., do you encounter any Issue tickets from GitHub Enterprise On-Premise users)

[gmlewis]

Should further preview header removals also be reverted? Or are we good with the current set of API Preview Headers? (e.g., do you encounter any Issue tickets from GitHub Enterprise On-Premise users)

I believe we are currently in a fine state in terms of GitHub Enterprise users. There were a couple requests a few months ago to revert the removals of specific headers, and we did so. To my knowledge, there are no currently outstanding requests for providing missing custom headers.

[andygrunwald]

Superseded by #2188