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

use correct http status code for rate limiting #1935

Merged
merged 1 commit into from Dec 1, 2017

Conversation

Projects
None yet
4 participants
@cemeyer2
Contributor

cemeyer2 commented Jul 29, 2015

403 is not the ideal status code to return when being rate limited/throttled. 429 is more semantically correct

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Jul 29, 2015

Member

Agreed. I'm postponing the merging of this PR because it introduces a new translatable string, but that is 👍

Member

ozh commented Jul 29, 2015

Agreed. I'm postponing the merging of this PR because it introduces a new translatable string, but that is 👍

@ozh ozh added this to the 1.8 milestone Jul 29, 2015

@LeoColomb

This comment has been minimized.

Show comment
Hide comment
@LeoColomb

LeoColomb Aug 6, 2015

Member

Big 👍

Member

LeoColomb commented Aug 6, 2015

Big 👍

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Apr 24, 2017

Coverage Status

Changes Unknown when pulling 68df396 on cemeyer2:cemeyer2-proper-http-codes into ** on YOURLS:master**.

coveralls commented Apr 24, 2017

Coverage Status

Changes Unknown when pulling 68df396 on cemeyer2:cemeyer2-proper-http-codes into ** on YOURLS:master**.

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Apr 24, 2017

Member

(nice job coveralls... 18 months late and analysed the wrong project :) )

Member

ozh commented Apr 24, 2017

(nice job coveralls... 18 months late and analysed the wrong project :) )

@LeoColomb

This comment has been minimized.

Show comment
Hide comment
@LeoColomb

LeoColomb Nov 29, 2017

Member

I think this can be merged, even if it introduces a new translatable string.
Actually, statuses for API should not be translated.

Member

LeoColomb commented Nov 29, 2017

I think this can be merged, even if it introduces a new translatable string.
Actually, statuses for API should not be translated.

@ozh ozh merged commit 68734d4 into YOURLS:master Dec 1, 2017

4 checks passed

Scrutinizer No new issues
Details
codacy/pr Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 99.923%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment