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

Add custom exception for the HTTP 451 response #1239

Merged
merged 5 commits into from
Apr 10, 2016

Conversation

devkhan
Copy link
Contributor

@devkhan devkhan commented Apr 4, 2016

Implements #1237.

As per this blog post.

GitHub API will now return a HTTP 451 response (rather than a 403) when a repository or gist cannot be accessed due to a DMCA notice.

Checklist:

  • Add new exception.
  • Add it to HandleErrors.
  • .\build FixProjects
  • Unit tests.
  • Integration tests.

/cc @ryangribble

@shiftkey
Copy link
Member

shiftkey commented Apr 4, 2016

@devkhan just added a task to ensure the files are synced across all the projects

@devkhan
Copy link
Contributor Author

devkhan commented Apr 9, 2016

@ryangribble The Travis Linux build is failing. Can you please review?

[Fact]
public void HasDefaultMessage()
{
var response = new Response(HttpStatusCode.Forbidden, null, new Dictionary<string, string>(), "application/json");
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldnt the status code here be the HTTP 451 (not Forbidden?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm really sorry for this, I'll fix this. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

No need to apologosie! Everyone's contributions are much appreciated 😀

@shiftkey shiftkey changed the title [WIP] Add custom exception for the HTTP 451 response Add custom exception for the HTTP 451 response Apr 10, 2016
@shiftkey
Copy link
Member

LGTM 👍

@shiftkey shiftkey merged commit cdd4646 into octokit:master Apr 10, 2016
@devkhan
Copy link
Contributor Author

devkhan commented Apr 10, 2016

Should I add some integration tests using one of the repos from http://github.com/github/dmca ?

@shiftkey
Copy link
Member

@devkhan don't worry about it. I think this area of the codebase is rather boring and unit tests are good enough coverage for the error code handling...

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