Skip to content

delete custom media type 'application/vnd.github.polaris-preview' #621

Merged
dmitshur merged 1 commit into
google:masterfrom
ahmedhagii:remove-custom-media-type-616
Jul 25, 2017
Merged

delete custom media type 'application/vnd.github.polaris-preview' #621
dmitshur merged 1 commit into
google:masterfrom
ahmedhagii:remove-custom-media-type-616

Conversation

@ahmedhagii
Copy link
Copy Markdown
Contributor

Fixes issue: #616

@ahmedhagii ahmedhagii force-pushed the remove-custom-media-type-616 branch from 76dd0e7 to 2d7aed2 Compare April 20, 2017 21:37
Comment thread github/repos.go
// https://developer.github.com/v3/licenses/#get-a-repositorys-license
acceptHeaders := []string{mediaTypeLicensesPreview, mediaTypeSquashPreview}
acceptHeaders := []string{mediaTypeLicensesPreview}
req.Header.Set("Accept", strings.Join(acceptHeaders, ", "))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given there's only one accept header value being set now, this can be simplified to just:

req.Header.Set("Accept", mediaTypeLicensesPreview)

How do you feel about that?

Comment thread github/repos_test.go Outdated
acceptHeader := []string{mediaTypeLicensesPreview}
mux.HandleFunc("/repos/o/r", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testHeader(t, r, "Accept", strings.Join(acceptHeader, ", "))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This too can be simplified to just:

testHeader(t, r, "Accept", mediaTypeLicensesPreview)

@dmitshur
Copy link
Copy Markdown
Member

Friendly ping @ahmedhagii. Did you see my review question above?

@dmitshur
Copy link
Copy Markdown
Member

Friendly ping @ahmedhagii.

If you prefer not to continue working on this, we can take this PR (which is 90% there), apply the suggested change above, and merge it. That's completely okay.

@dmitshur
Copy link
Copy Markdown
Member

I'm going to apply the suggested simplifications myself.

The Merge Methods API has become an official part of GitHub API v3,
so the 'application/vnd.github.polaris-preview' preview API media type
is no longer needed.

GitHub Announcement: https://developer.github.com/changes/2017-04-07-end-merge-methods-api-preview/

Fixes google#616.
@dmitshur dmitshur force-pushed the remove-custom-media-type-616 branch from 2d7aed2 to 59ae030 Compare July 25, 2017 15:47
@dmitshur
Copy link
Copy Markdown
Member

The simplifications were no longer applicable due to the changes that have since happened. I've rebased, resolved the merge conflict, improved the commit message, and updated the PR.

Copy link
Copy Markdown
Member

@dmitshur dmitshur left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@dmitshur dmitshur merged commit 35d3810 into google:master Jul 25, 2017
nbareil pushed a commit to nbareil/go-github that referenced this pull request May 1, 2018
The Merge Methods API has become an official part of GitHub API v3,
so the 'application/vnd.github.polaris-preview' preview API media type
is no longer needed.

GitHub Announcement: https://developer.github.com/changes/2017-04-07-end-merge-methods-api-preview/

Fixes google#616.
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