Skip to content

Conversation

@alexandear
Copy link
Contributor

Recently, I found PR #2188, which suggests that we should keep custom Accept headers for preview APIs.

There's also a comment related to this:

// Media Type values to access preview APIs.
// These media types will be added to the API request as headers
// and used to enable particular features on GitHub API that are still in preview.
// After some time, specific media types will be promoted (to a "stable" state).
// From then on, the preview headers are not required anymore to activate the additional
// feature on GitHub.com's API. However, this API header might still be needed for users
// to run a GitHub Enterprise Server on-premise.
// 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 the header promotion is going out for GitHub.com, it may be some time before it
// even arrives in GitHub Enterprise Server.
// We keep those preview headers around to avoid breaking older GitHub Enterprise Server
// versions. Additionally, non-functional (preview) headers don't create any side effects
// on GitHub Cloud version.
//
// See https://github.com/google/go-github/pull/2125 and https://github.com/google/go-github/pull/2188 for full context.

Therefore, this PR reverts my recent change in PR #3820 (commit 3959138) and removes misleading TODO comments suggesting the removal of custom Accept headers.

@gmlewis
Copy link
Collaborator

gmlewis commented Nov 13, 2025

Ah yes, I'm very sorry for forgetting about that. My bad. Thank you, @alexandear!

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Merging.

@gmlewis gmlewis merged commit 025030e into google:master Nov 13, 2025
5 checks passed
@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.33%. Comparing base (8657c38) to head (f066860).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3824   +/-   ##
=======================================
  Coverage   92.32%   92.33%           
=======================================
  Files         194      194           
  Lines       13979    13990   +11     
=======================================
+ Hits        12906    12917   +11     
  Misses        884      884           
  Partials      189      189           

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants