Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accepts parameter for Patch #440

Merged
merged 3 commits into from
Apr 16, 2014
Merged

Conversation

nigel-sampson
Copy link
Contributor

This is a bit of a weird pull request, the app I'm building has Markdown support, so I'm using the underlying ApiConnection class to make use of Octokit, but with a custom accepts header. For instance:

public static Task<FullIssue> CreateFullIssueAsync(this IGitHubClient gitHubClient, string owner, string name, NewIssue issue)
{
    var uri = ApiUrls.Issues(owner, name);

    var apiConnection = new ApiConnection(gitHubClient.Connection);

    return apiConnection.Post<FullIssue>(uri, issue, "application/vnd.github.v3.full+json; charset=utf-8");
}
public class FullIssue : Issue
{
    public string BodyText
    {
        get; set;
    }

    public string BodyHtml
    {
        get; set;
    }
}

However some of the methods don't have overloads that take an accepts parameter. This will add them.

@shiftkey
Copy link
Member

shiftkey commented Apr 2, 2014

Am I understanding this right - you're posting to a custom API with this FullIssue entity, which is then propagated onto the GitHub API? With the Markdown and the rendered HTML?

@nigel-sampson
Copy link
Contributor Author

No, I'm posting to the GitHub API directly. By changing the accepts header I can post the markdown, and when getting issues get both the markdown and html in one request.

In the above code I'm making use of the Octokit connection and parsing etc but need to send the custom accepts header.

However some of the underlying methods for Patch and Get don't expose an overload that takes accepts.

@shiftkey
Copy link
Member

shiftkey commented Apr 2, 2014

Right, I misread the context. My bad.

This all seems rather legit. Is it worth throwing any tests into the mix?

@nigel-sampson
Copy link
Contributor Author

Done :-)

haacked added a commit that referenced this pull request Apr 16, 2014
@haacked haacked merged commit 9e489b6 into octokit:master Apr 16, 2014
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.

3 participants